If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

about: document.writes userAgent without escaping

VERIFIED DUPLICATE of bug 95768

Status

()

Core
Security
P2
normal
VERIFIED DUPLICATE of bug 95768
16 years ago
16 years ago

People

(Reporter: Jesse Ruderman, Assigned: Mitchell Stoltz (not reading bugmail))

Tracking

Trunk
mozilla0.9.6
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: patch, PDT-, URL)

Attachments

(1 attachment)

(Reporter)

Description

16 years ago
bbaetz noticed this bug, but I don't think he reported it yet.

about: document.writes userAgent without escaping it, and also document.writes
part of userAgent as the version number.  This is problematic for two reasons:

1. The user may have set Mozilla to spoof Internet Explorer's userAgent.  about:
will then report that Mozilla is Internet Explorer.

2. about: calls document.write directly on navigator.userAgent instead of
escaping the userAgent string first.  Thus, any HTML tags in the will be
interpreted as HTML rather than displayed as they are written in the userAgent.
 Worse, the URL about: is mapped to chrome URL, so any scripts in these tags
will run with chrome privileges.

I know of at least one current security hole that would allow a web page to
change the navigator.userAgent that about: sees. Also, if someone has write
access to prefs.js, they can change navigator.useragent.override, effectively
changing navigator.userAgent, and they can also change the home page to about:
to ensure that their code will be run.

I can fix (2) quickly by adding an htmlEscape function to about:.
(Reporter)

Comment 1

16 years ago
Created attachment 41997 [details] [diff] [review]
fixes the security hole
(Reporter)

Comment 2

16 years ago
Note that about.html is in the en-US directory, so this change might affect
internationalization, even though I didn't change any text.

Comment 3

16 years ago
Adding Frank Tang on the cc list.  Frank, how do we ensure that this fix applies
to about.html of all the various locales?

Marking nsBranch...
Keywords: nsBranch

Comment 4

16 years ago
Really adding Frank to the cc list.  Frank, how do we ensure that this fix 
applies to about.html of all the various locales?  Thanks for your help!

Comment 5

16 years ago
>I know of at least one current security hole that would allow a web page to
>change the navigator.userAgent that about: sees. Also, if someone has write
>access to prefs.js, they can change navigator.useragent.override, effectively
>changing navigator.userAgent, and they can also change the home page to about:
>to ensure that their code will be run.

Jesse, which security hole would allow a web page to change navigator.userAgent? 
 Shouldn't we try to fix that security hole?

Also, presumably, only the user of a machine has write access to the prefs.js on 
that machine.  Is there any way that the setting of navigator.useragent.override 
and the resetting of the home page can happen as a user is browsing the web?  If 
not, then these two attacks are not ship stoppers, right?
(Reporter)

Updated

16 years ago
Priority: -- → P2
Target Milestone: --- → mozilla0.9.3
(Assignee)

Comment 6

16 years ago
We have a couple of attacks with fixes pending that allow an attacker to set
properties of a window in another domain, like "about:". The attacker could
redefine navigator.userAgent via the DOM. This doesn't require write access to
prefs.js at all. So in answer to your first question, we are fixing those other
holes we know of, but there may be more. By itself, being able to set a property
in another domain is a pretty serious exploit, but combined with this bug, it
becomes a complete system compromise exploit. We hope we've fixed all
same-domain exploits, but we should also prevent any undiscovered same-domain
exploits from becoming full-compromise exploits by fixing this bug.

Comment 7

16 years ago
approved the change to about.html, we will update the file in the localized
builds as usual. Copying mcarlson and danielmc
(Assignee)

Comment 8

16 years ago
r=mstoltz.

Comment 9

16 years ago
sr=blake
(Reporter)

Comment 10

16 years ago
Crap, my patch doesn't fix the security hole.  An attacker with a cross-domain
exploit could set navigator.userAgent to an Object instead of a string, and have
the object define .replace and .toString in such a way that htmlEscape returns a
string containing chars such as <>&.

I asked Phil how to get around that problem, and he suggested using the String
constructor: 
s = String(s);

That worked for about 30 minutes until I realized that all the attacker would
have to do to get around that is to redefine "String" to be the identity
function at the same time they redefined "navigator.userAgent" to be the evil
object.

cc brendan and phil.  Any ideas?

Comment 11

16 years ago
I have spoken to Jesse about this issue. My only suggestion was that
functionality like String() (which can be altered) might be put as a
read-only property of some security object. That way, one could use it 
without worrying if it had been altered by the attacker - 

Jesse pointed out that getters/setters on it would need to be guarded
against, as well - 
(Reporter)

Comment 12

16 years ago
See also bug 88087, about: shouldn't have chrome privs.
Why should scripts be able to redefine navigator.userAgent (or any other
navigator. property)? In 4.x these were all read-only properties and I see no
good reason to allow them to be set.
(Reporter)

Comment 14

16 years ago
Until recently, there was a bug that web pages could change the
navigator.userAgent seen by about: (bug 87389).  But even without that bug,
Mozilla lets users change their UA string, and that shouldn't affect the
information shown in about:.
well you could hardcode the version into about.html (and maybe pickup the Gecko 
builddate which is compiled-in IIRC), but then we've got yet another thing 
someone has to remember to change for each milestone release.

We could preface the user agent display with a label saying "Current user 
agent:" -- then if the user has a spoofed UA you call it a service letting 
them know how they appear to the 'net, and for the vast majority of people it's 
the correct current version.

Comment 16

16 years ago
why should we care if we display what someone has set the user agent to vs what
the standard shipping browser displays as useragent?  I would think that we want
to display the useragent in the about page as whatever it's set to, otherwise
how would people know what user agent their browser is using?

I also was under the impression that navigator.useragent was read-only (with the
exception of people customizing it themselves).

Yes, we could hard code the user agent information in the about page, but the
whole reason we went dynamic was because it was always out of date.  So if we go
back to hard coding for security reasons, know that the price will be never
having an up to date user agent string.
(Assignee)

Comment 17

16 years ago
Instwad of changing the way the useragent string is displayed, let's try not
using the system principal for the about: page. This may break something, but
it's worth a try and would make things a lot safer.
(Reporter)

Updated

16 years ago
Target Milestone: mozilla0.9.3 → mozilla0.9.4
(Reporter)

Comment 18

16 years ago
Let's check in the htmlEscape fix anyway.  At least it makes us display the
right thing (and not be vulnerable to a security hole) when the user agent is
set to some wacky value via prefs.js.
(Reporter)

Comment 19

16 years ago
See bug 94551 for another about: page that needs to escape text.

Comment 20

16 years ago
nsbranch+ per pdt triage
it would be good to get it if we can find a fix within next two weeks
assigning to mstoltz 
Assignee: jruderman → mstoltz
Keywords: nsbranch → nsbranch+
(Assignee)

Comment 21

16 years ago
I'll take this, to make sure it gets done for the Netscape branch.
Keywords: nsbranch+ → nsbranch
Keywords: nsbranch → nsbranch+

Comment 22

16 years ago
0.9.4 is out the door.
Target Milestone: mozilla0.9.4 → mozilla0.9.5

Comment 23

16 years ago
mitch - still important for the branch?
QA Contact: ckritzer → bsharma
(Assignee)

Comment 24

16 years ago
I don't think this is important for the branch; there's no security risk at this
point. I'll check it in on the trunk.
Status: NEW → ASSIGNED
Keywords: nsbranch+
Whiteboard: patch

Comment 25

16 years ago
PDT- for the branch according to Mitch's comments. 
Whiteboard: patch → patch, PDT-
(Assignee)

Comment 26

16 years ago
time marches on...retargeting to 0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6

Comment 27

16 years ago
I were doing a rewrite of about: in XHTML (bug 95768). And I just noticed this bug.

To make the page a correct working XML document I changed the javascript code to
the following:

<td id="mozver">
<h1>
<a id="mozlink" href="http://www.mozilla.org/releases/">Mozilla </a>
</h1>
<script type="application/x-javascript">
 
document.getElementById("mozlink").firstChild.appendData(navigator.userAgent.match(/rv:([^;)]+)/)[1]);
 
document.getElementById("mozver").appendChild(document.createTextNode(navigator.userAgent));
</script>
</td>


Can this code be an alternative solution to the problem, since the userAgent
string is put directly into DOM Text object?
Can the contents of a Text object still become another elements/objects?
(Assignee)

Comment 28

16 years ago
Looks like a viable alternative solution to me, but I'm not sure. Jesse? jst?

Comment 29

16 years ago
I've attached a patch in bug 95768 which incorporates the above changes and
removes the previous fix of this bug.

You are welcome to review it. :o)
Looks reasonable to me too.
(Assignee)

Comment 31

16 years ago
I'd like to hear from Jesse before we check this in, as he's got a good eye for
how things can be subverted.
(Reporter)

Comment 32

16 years ago

*** This bug has been marked as a duplicate of 95768 ***
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → DUPLICATE

Comment 33

16 years ago
vrfy
Status: RESOLVED → VERIFIED
Do we really want the version shown by About to depend on the
User Agent?  I noticed this when I changed my UserAgent to
spoof Netscape 6.2 to verify sniffing in conjunction with
bug 111026.  I changed the UA string, and whammo, Mozilla
0.9.6 became Mozilla 0.9.4, as far as About was concerned.
  
It doesn't seem so wrong that the User Agent string shown 
in the fine print, but it would be nice if the version shown 
in the header were accurate.  That may, however, require 
backend support, or else need to be changed in the About
html each version.  Say, can the build process write it 
into the html at compile time?

In any event, bug 95768 has apparently been resolved 
without changing this.  Are we getting lost in the
shuffle here, or am I totally misunderstanding what
this bug is about?

Comment 35

16 years ago
I do, how else do we remind users that they're actively forging their 
useragent?
> how else do we remind users that they're actively forging 

This might be an issue if there were a UI for changing such
things, but with it being restricted to only users who edit
prefs.js by hand, I expect just about everyone who is 
spoofing knows he is spoofing.  

Anyway, just having the entire UA string shown in About
is no problem as far as I'm concerned.  It's that the
about box in Mozilla 0.9.6 says "Mozilla 0.9.4" in
enormous print and *doesn't say 0.9.6 anyplace*, that's 
what bothers me.  As it stands now, the only way to 
determine the actual version, short of removing the 
spoofing, is to use the build number, but that doesn't 
carry trunk/branch information and so is not a complete 
answer.  So the only real way to find out which version 
you are using is to edit prefs.js, but that requires
exiting Mozilla -- this is a very real problem for users 
who have multiple versions installed for one reason or 
another.  

I don't mind that the spoofed info is shown in About; 
what I do mind is that the truth is not also  shown along 
with it.  The user needs to be able to get this information, 
preferably without editing prefs.js every time.

Websites don't actually need to know what browser the 
user is using.  The user does need to know that.

Comment 37

16 years ago
no, we are not making the mozilla about page static again.  The reason it's
dynamic is so that you don't have to change it every 5 weeks for a new release.
 If you're running multiple versions of mozilla and savvy enough to spoof user
agents then you have to be savvy enough to remember it later.  If you want to
find out what version you're really running, you can go in and unspoof your ua,
and it will tell you.
You need to log in before you can comment on or make changes to this bug.