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

Possible XSS vulnerability #650

Closed
enferas opened this issue Jul 20, 2022 · 4 comments
Closed

Possible XSS vulnerability #650

enferas opened this issue Jul 20, 2022 · 4 comments

Comments

@enferas
Copy link

enferas commented Jul 20, 2022

Hello,

I would like to report for possible XSS vulnerability.

In file https://github.com/phoronix-test-suite/phoronix-test-suite/blob/master/pts-core/phoromatic/pages/phoromatic_r_add_test_details.php

\\line 41
// Note: the source
$test_profile = new pts_test_profile($_GET['tp']);

In file https://github.com/phoronix-test-suite/phoronix-test-suite/blob/master/pts-core/objects/pts_test_profile.php

public function __construct($identifier = null, $override_values = null, $normal_init = true){
	parent::__construct($identifier, $normal_init);
        //...
}

In file https://github.com/phoronix-test-suite/phoronix-test-suite/blob/master/pts-core/objects/pts_test_profile_parser.php

public function __construct($read = null, $normal_init = true){
//...
$this->xml = simplexml_load_string($read, 'SimpleXMLElement', $xml_options);
}

Now the input is in the xml property in object $test_profile.

In file https://github.com/phoronix-test-suite/phoronix-test-suite/blob/master/pts-core/phoromatic/pages/phoromatic_r_add_test_details.php

//line 47
if(!empty($supported_os = $test_profile->get_supported_platforms())){
        // Note: the sink
	echo '<p>This test is supported on <strong>' . implode(', ', $supported_os) . '</strong>.</p>';
}

In file https://github.com/phoronix-test-suite/phoronix-test-suite/blob/master/pts-core/objects/pts_test_profile_parser.php

public function get_supported_platforms_raw(){
		return $this->xg('TestProfile/SupportedPlatforms');
}

public function xg($xpath, $default_on_null = null){
		//...
		$r = $this->xml ? $this->xml->xpath($xpath) : null;
                //...
                return $r;
}

Thus the input will be controlled by the attacker, saved in the xml then extracted and printed through the variable $supported_os.

@michaellarabel
Copy link
Member

Unless you are on an outdated version, on the line prior to $test_profile = new pts_test_profile($_GET['tp']); is phoromatic_quit_if_invalid_input_found(array('tp'));. That phoromatic_quit_if_invalid_input_found() call will bail if that variable has any < > characters, among other characters, so XML cannot be passed ultimately on that line... Unless you found some way to bypass that, that line has already prevented the possibility of XSS there.

@enferas
Copy link
Author

enferas commented Jul 23, 2022

Hello,

Thank you for your response.
Yes it is true that I didn't see the function phoromatic_quit_if_invalid_input_found where it exit the project if there are special characters.

Anyway I think I found a way to bypass this check.
In phoromatic_quit_if_invalid_input_found your are checking the $_REQUEST which can carry the value of the $_POST and the $_GET.

function phoromatic_quit_if_invalid_input_found($input_keys = null)
{
	if(empty($input_keys))
	{
		// Check them all if not being selective about what keys to check
		$input_keys = array_keys($_REQUEST);
	}
	// backup as to sanitization and stripping elsewhere, safeguard namely check for things like < for fields that shouldn't have it
	// plus a few simple backups as safeguards for words that really have no legit relevance within Phoromatic...

	foreach(pts_strings::safety_strings_to_reject() as $invalid_string)
	{
		foreach($input_keys as $key)
		{
			if(isset($_REQUEST[$key]) && !empty($_REQUEST[$key]))
			{
				foreach(pts_arrays::to_array($_REQUEST[$key]) as $val_to_check)
				{
					if(stripos($val_to_check, $invalid_string) !== false)
					{
						echo '<strong>Exited due to invalid input ( ' . $invalid_string . ') attempted:</strong> ' . htmlspecialchars($val_to_check);
						exit;
					}
				}
			}
		}
	}
}

What I can do here that sending the post and the get with the same key (tp). I pass the safe value with the $_POST and the injection with the $_GET. Thus phoromatic_quit_if_invalid_input_found will check the value in the $_POST which is safe and I continue my injection with $_GET.

Here just a sample example I create to show that $_REQUEST is printing the value of the $_POST.

//server code
<?php

echo $_GET['tp']; // print getxxxx
echo $_POST['tp']; // print postyy
echo $_REQUEST['tp']; // print postyy
//payload
<?php
?>
<html>
<body>
<form action="http://localhost:8082/server.php?tp=getxxxx" id="myform" method="post" name="myform">
    <input type="hidden" name="tp" value="postyy">
    <input type="submit" value="Add Action" id="clickButton">
  </form>

  </body></html>
  <script>
  window.onload = function(){
    var button = document.getElementById('clickButton');
    button.form.submit();
}
</script>

looking forward for your response.

michaellarabel added a commit that referenced this issue Jul 23, 2022
@enferas
Copy link
Author

enferas commented Jul 23, 2022

Thank you @michaellarabel for applying this fix. This fix solve multiple reports that I had.
I am considering the fix as a confirmation about my report and I will apply for a CVE which will help me in my research.

@enferas
Copy link
Author

enferas commented Dec 30, 2022

CVE-2022-40704 is assigned to this report.

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