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

Various XSS issues with HTML Forms handling #4035

Closed
YongBoLiu opened this issue Jan 6, 2021 · 13 comments
Closed

Various XSS issues with HTML Forms handling #4035

YongBoLiu opened this issue Jan 6, 2021 · 13 comments
Labels
bug Undesired behaviour resolved A fixed issue SECURITY A security issue reported through CVE
Milestone

Comments

@YongBoLiu
Copy link
Contributor

YongBoLiu commented Jan 6, 2021

Describe the bug

A clear and concise description of what the bug is.

XSS Defect
https://hostname/cacti/plugins.php
https://hostname/cacti/settings.php
https://hostname/cacti/data_templates.php
https://hostname/cacti/graph_templates_inputs.php

To Reproduce

Steps to reproduce the behavior:

  1. The Web Application Report was created by HCL AppScan Standard 9.0.3.14 iFix001, Rules: 21006
  2. See the pictures attached below.
@YongBoLiu YongBoLiu added bug Undesired behaviour unverified Some days we don't have a clue labels Jan 6, 2021
@TheWitness
Copy link
Member

@YongBoLiu are you planning logging a CVE for these?

@TheWitness
Copy link
Member

Also, what version was this against? I'm not seeing some of these in the code.

@TheWitness
Copy link
Member

TheWitness commented Jan 6, 2021

  1. Content-Security-Policy Stored XSS is acknowledged should be medium as it requires and injection vector first
  2. data_templates.php can not be reproduced. You may have old code.
  3. plugins.php is just a repeat of 1)
  4. settings.php is the same as 1)

So, I can see two issues from the images, and they are stored.

@TheWitness
Copy link
Member

Retest, but confirm that you have the lastest data_templates.php in your test as the section with the XSS is already escaped in the 1.2.x code.

@TheWitness TheWitness added this to the v1.2.17 milestone Jan 6, 2021
@TheWitness
Copy link
Member

Deleted the images for you. Pretty sure this is resolved now.

@TheWitness TheWitness added resolved A fixed issue SECURITY A security issue reported through CVE and removed unverified Some days we don't have a clue labels Jan 6, 2021
@YongBoLiu
Copy link
Contributor Author

@TheWitness Thanks for changed the info. And the scan was done on 11/18/2020 base on 1.2.15 before our FP11 release.

@YongBoLiu
Copy link
Contributor Author

YongBoLiu commented Jan 6, 2021

For the data_templates.php, base on the report, I can set the name to aonmouseover=alert(102) as the report shows.
From the code level, it concern about the code below,

				<td class='textInfo left' style='vertical-align:top;'>
					<?php print html_escape($template['name']);?>
				</td>
				<td class='textInfo right' style='vertical-align:top;'>

The html_escape() cannot escape the Grave Accent ` char, which is need refer to some links below,
https://wonko.com/post/html-escaping/
https://davidmurdoch.com/2017/09/02/the-grave-accent-and-xss/

So, I think the html_escape() in the ./lib/html.php should change as below,

--- /home/yboliu/my_git/rtm/rtm/cacti/./lib/html.php	2021-01-06 15:39:21.469490865 +0800
+++ ./lib/html.php	2021-01-06 15:13:50.576283881 +0800
@@ -1041,7 +1041,7 @@
 		$charset = 'UTF-8';
 	}
 
-	return htmlspecialchars($string, ENT_QUOTES, $charset, false);
+	return htmlentities($string, ENT_QUOTES|ENT_HTML5, $charset, false);
 }
 
 /* html_split_string - takes a string and breaks it into a number of <br> separated segments

@YongBoLiu
Copy link
Contributor Author

YongBoLiu commented Jan 6, 2021

The tested code is below,

<?php
//var_dump(get_html_translation_table(HTML_ENTITIES, ENT_QUOTES | ENT_HTML5));

echo "Test String:\n";
echo("<td class='textInfo left' style='vertical-align:top;'>`a`onmouseover=`alert(102)`</td>");
echo "\n";
echo "htmlspecialchars\n";
$new = htmlspecialchars("<td class='textInfo left' style='vertical-align:top;'>`a`onmouseover=`alert(102)`</td>", ENT_QUOTES);
echo $new;
echo "\n";
echo "htmlspecialchars with ENT_HTML5\n";
$new = htmlspecialchars("<td class='textInfo left' style='vertical-align:top;'>`a`onmouseover=`alert(102)`</td>", ENT_QUOTES | ENT_HTML5);
echo $new;
echo "\n";
echo "htmlentities\n";
$new = htmlentities("<td class='textInfo left' style='vertical-align:top;'>`a`onmouseover=`alert(102)`</td>", ENT_QUOTES);
echo $new;
echo "\n";
echo "htmlentities with ENT_HTML5\n";
$new = htmlentities("<td class='textInfo left' style='vertical-align:top;'>`a`onmouseover=`alert(102)`</td>", ENT_QUOTES | ENT_HTML5);
echo $new;
?>

And the test result is below,

[root@xianrtm tmp]# php ./a.php
Test String:
<td class='textInfo left' style='vertical-align:top;'>`a`onmouseover=`alert(102)`</td>
htmlspecialchars
&lt;td class=&#039;textInfo left&#039; style=&#039;vertical-align:top;&#039;&gt;`a`onmouseover=`alert(102)`&lt;/td&gt;
htmlspecialchars with ENT_HTML5
&lt;td class=&apos;textInfo left&apos; style=&apos;vertical-align:top;&apos;&gt;`a`onmouseover=`alert(102)`&lt;/td&gt;
htmlentities
&lt;td class=&#039;textInfo left&#039; style=&#039;vertical-align:top;&#039;&gt;`a`onmouseover=`alert(102)`&lt;/td&gt;
htmlentities with ENT_HTML5
&lt;td class&equals;&apos;textInfo left&apos; style&equals;&apos;vertical-align&colon;top&semi;&apos;&gt;&grave;a&grave;onmouseover&equals;&grave;alert&lpar;102&rpar;&grave;&lt;&sol;td&gt;

It works well on data_templates.php (Console->Template->Data Source) page. It is escaped and shows well in the GUI when input the name as below,

`a`onmouseover=`alert(102)`

@TheWitness
Copy link
Member

Okay, I'll make that change this morning.

TheWitness added a commit that referenced this issue Jan 6, 2021
XSS security issue from WebScan report
TheWitness added a commit that referenced this issue Jan 6, 2021
This is based upon recommendations. to pre-sanitize the referer always and we don't want to double sanitize.
@YongBoLiu
Copy link
Contributor Author

@TheWitness Base on the test result, the htmlspecialchars() should be htmlentities(), then the Grave Accent ` char can be escaped.

@TheWitness
Copy link
Member

Oh, did not see that. Make the change and submit a pull request. My fingers are numb.

YongBoLiu pushed a commit to YongBoLiu/cacti that referenced this issue Jan 7, 2021
@YongBoLiu YongBoLiu mentioned this issue Jan 7, 2021
TheWitness pushed a commit that referenced this issue Jan 7, 2021
* feature 3965, Add a value check if the input field is textbox in settings

* feature 3965, revert

* Fix part of #4035, XSS security issue from WebScan report

Co-authored-by: yboliu <yboliu@oc8636837557.ibm.com>
@TheWitness
Copy link
Member

Okay, using a better, more compatible solution now. This one passes all my tests.

@YongBoLiu
Copy link
Contributor Author

That's great! Thanks.

@netniV netniV changed the title XSS security issue from WebScan report Various XSS issues with HTML Forms handling Apr 12, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jul 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour resolved A fixed issue SECURITY A security issue reported through CVE
Projects
None yet
Development

No branches or pull requests

2 participants