412437 - if an admin has saved his password on doctor.mozilla.org bad things may happen

Status: RESOLVED FIXED

(Webtools Graveyard :: Doctor, defect)

Description

[georgi - hopefully not receiving bugspam]

xss allows stealing saved passwords.
doctor.mozilla.org allows xss by design =>

saved doctor password may be stolen when visiting malicious page (direct url, hidden iframe)

this example:
https://doctor.mozilla.org/?action=regurgitate&file=mozilla-org/html/news.html&content=%3Cform%20id=%22form%22%20method=%22post%22%20action=%22doctor.cgi%22%20enctype=%22multipart/form-data%22%3E%3Cscript%3Ealert(5)%3C/script%3E%3Cinput%20type=%22text%22%20name=%22username%22%20size=%2220%22%3E%3Cinput%20type=%22password%22%20name=%22password%22%20size=%2220%22%3E%3C/form%3E

prefills the saved values (if any) and executes javascript

note that any saved valid password allows defacing all of m.o website if malicious url is visited.

Comment 1

[timeless]

myk added that feature (1.13)

Comment 2

[georgi - hopefully not receiving bugspam]

Bug 408531 and its duplicates discuss password manager in suite

Comment 3

[georgi - hopefully not receiving bugspam]

as i have written, doctor.cgi should be rm(1)ed

in retrieve():  print $request->header(
        -type                =>  "text/html; name=\"" . $file->name . "\"",

no validation, "\x00" makes cvs happy and arbitrary content may be appended via file->name

this gives 500
https://doctor.mozilla.org/?action=display&file=mozilla-org/html/news.html%00%0a%0a%0ahhhhhh&content=%3Cform%20id=%22form%22%20method=%22post%22%20action=%22doctor.cgi%22%20enctype=%22multipart/form-data%22%3E%3Cscript%3Ealert(5)%3C/script%3E%3Cinput%20type=%22text%22%20name=%22username%22%20size=%2220%22%3E%3Cinput%20type=%22password%22%20name=%22password%22%20size=%2220%22%3E%3C/form%3E

Comment 4

[Frédéric Buclin]

To be sure: in order to steal your mozilla.org password, the attacker would need an account on mozilla.org to hack the HTML file you are viewing, right? In this case, why would he need your password as he already can edit the website?

Comment 5

[georgi - hopefully not receiving bugspam]

(In reply to comment #4)
> To be sure: in order to steal your mozilla.org password, the attacker would
> need an account on mozilla.org to hack the HTML file you are viewing, right? In
> this case, why would he need your password as he already can edit the website?
> 

no. you are misunderstanding.

check the uri in the description and in the URL field.

doctor.cgi's |action=regurgitate| just echoes the input from the url with content type text/html

comment #3 is another potential xss.

if the password on doctor.m.o is saved and js is enabled, the password can be stolen from anywhere, no m.o accounts are needed

Comment 6

[georgi - hopefully not receiving bugspam]

[sg:high?]

Comment 7

[Myk Melez [:myk] [@mykmelez]]

It's not clear who is responsible for this site these days.  Cc:ing folks from IT and webdev.

It's also not clear what kind of validation we should be doing on CVS filenames, per comment 3.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Attachment: workaround: disables regurgitation

Here's a workaround for the first problem that simply disables regurgitation until it can be made secure.

Comment 9

[georgi - hopefully not receiving bugspam]

(In reply to comment #7)
>
> It's also not clear what kind of validation we should be doing on CVS
> filenames, per comment 3.
> 

for start aborting if the string contains "\x00" (zero byte) probably will be enough

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: fixes described issues

This should do it.  Dave, if this looks ok to you, then someone from IT should deploy this to doctor.mozilla.org and make sure it works there as well before we check it in and make this bug public.

Comment 11

[Dave Miller [:justdave]]

Comment on attachment 302553 [details] [diff] [review]
patch v2: fixes described issues

Looks reasonable.  I'll give it a shot in production before signing off on it though.

I'd imagine we could probably get around this by using a different domain name for regurgitate and ensure that we can't display the uploaded file via the normal domain name.

Comment 12

[Dave Miller [:justdave]]

Comment on attachment 302553 [details] [diff] [review]
patch v2: fixes described issues

OK, got this deployed on https://doctor-test.mozilla.org/ -- seems to work as designed.  Pushing it to production now.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

Thanks Dave.

Checking in doctor.cgi;
/cvsroot/mozilla/webtools/doctor/doctor.cgi,v  <--  doctor.cgi
new revision: 1.27; previous revision: 1.26
done
Checking in doctor.js;
/cvsroot/mozilla/webtools/doctor/doctor.js,v  <--  doctor.js
new revision: 1.5; previous revision: 1.4
done
Checking in Doctor/File.pm;
/cvsroot/mozilla/webtools/doctor/Doctor/File.pm,v  <--  File.pm
new revision: 1.13; previous revision: 1.12
done