Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support request: Potential XSS found with #value on $raw_form_input #46

Open
hkirsman opened this issue Oct 21, 2019 · 1 comment
Open

Comments

@hkirsman
Copy link

Could somebody say what to do about this error:

------------------------------------------------------------------------------------------------------------------------
 66 | WARNING | Potential XSS found with #value on $raw_form_input
    |         | (PHPCS_SecurityAudit.Drupal7.XSSFormValue.D7XSSWarFormValue)
------------------------------------------------------------------------------------------------------------------------

The error is coming from where the #value' is red:

function mymodule_file_move_form_submit_handler(array &$form, FormStateInterface $form_state) {

	// Value from form select.
	$raw_form_input = Xss::filter($form['uri_scheme']['markup']['#value']);
	$uri_scheme = Html::escape($raw_form_input);

There's no other key I could use to get the value.

@jmarcil
Copy link
Collaborator

jmarcil commented Feb 18, 2020

This tool generally gives a WARNING when it thinks that something is a potential issue.

In your case you are using some type of filtering functions that the tool doesn't recognize.

For Drupal it's actually:

'check_plain', 't', 'l', 'url', 'drupal_attributes', 'drupal_render_children', 'drupal_render'

https://github.com/FloeDesignTechnologies/phpcs-security-audit/blob/master/Security/Sniffs/Drupal7/Utils.php#L31

Currently the tool doesn't support customization of the sort to remove false positive, but it's certainly a feature that would be awesome to have.

You could technically add your function to that list I stated above, but right now the sniff code path for it doesn't check mitigation: https://github.com/FloeDesignTechnologies/phpcs-security-audit/blob/master/Security/Sniffs/Drupal7/XSSFormValueSniff.php#L40

If you wanna hack that file too then with both hacks together it will dismiss the warning:

			if ($next == $closer && $tokens[$next]['code'] == T_SEMICOLON)  {
				$funcprv = $phpcsFile->findPrevious(T_OPEN_PARENTHESIS, $next);
				if (!in_array($tokens[$funcprv - 1]['content'], $utils::getXSSMitigationFunctions())) {
					// Case of $label = $element['#value'];
					$next = $phpcsFile->findPrevious(\PHP_CodeSniffer\Util\Tokens::$assignmentTokens, $next);
					$next = $phpcsFile->findPrevious(T_VARIABLE, $next);
					$phpcsFile->addWarning('0Potential XSS found with #value on ' . $tokens[$next]['content'], $next, 'D7XSSWarFormValue');
				}

I think we should open 2 issues following your comment:

  • One for the bug in Drupal7\XSSFormValueSniff of not checking getXSSMitigationFunctions on the Case of $label = $element['#value']; (and we can only do after after we prove that a #value is considered safe after a check_plain and the others)
  • One for the feature or adding custom mitigation functions

Let me know if this would solve your current concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants