all copies of shexp.c are vulnerable



9 years ago
8 months ago


(Reporter: nelson, Assigned: dveditz)



1.9.1 Branch

Firefox Tracking Flags

(blocking1.9.1 .2+, status1.9.1 .2-fixed)


(Whiteboard: [sg:dupe 332173][Embargoed until at least 2009-07-29])

** Embargoed until 2009-07-29 1800 UTC **

To resolve bug 498500, copies of shexp.c were copied/committed to 
modules/libjar/nsWildCard.cpp  and

If the pattern passed to these functions can come from an external source
(e.g., from a web page or other downloaded content), they are vulnerable 
to code execution attacks.  See bug 504456 for details.  

It would be good if we could find a single fix for bug 504456 and this bug.
that is, it would be good to find a single patch for all copies.

Assigning to Robert, who committed those copies.  Robert, I will let you determine which, if any, of these copies are actually vulnerable.


9 years ago
Whiteboard: [Embargoed until 2009-07-29]

Comment 1

9 years ago
Thanks for filing this after our IM chat, Nelson. The embargo date assumes 1) the original reporter will mention this at BlackHat on that date and 2) has noticed that the reported problem extends to libjar and the filepicker.

I'm pretty sure bug 498500 has nothing (or little) to do with this problem. These files are used on the 1.8 branch, for instance:
(the libjar copy goes back to 1999)

The xpfe filepicker moved to toolkit for 1.9.1

and consolidated into xpcom/io on trunk
Assignee: kairo → dveditz
status1.9.1: --- → needstriage
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Flags: blocking1.9.0.13?
Whiteboard: [Embargoed until 2009-07-29] → [Embargoed until at least 2009-07-29]

Comment 2

9 years ago
We look OK on the libjar front, all uses of nsIZipReader.findEntries are specific files or simple baked-in "*" wildcard patterns (e.g. looking for files of a specific extension). Checking a large sampling of Addons most uses were similar, although a few had a pattern variable and we'd have to review them to make sure the pattern didn't come from user input.
NSS's copy goes back to 1994. :)

I got the reference to bug 498500 from these pages:
Maybe those pages are lying to me about the bug that is relevant to their checkins (wouldn't surprise me).

I wrote a patch for this shexp code and attached it to bug 504456. 
I think it plugs the vulnerability, but I'm not happy with it.  
That shexp code is a real piece of work.  It doesn't handle nesting or 
escaping properly.  Fixing it the right way would be a rewrite.  NSS doesn't
need it badly enough to warrant that, but maybe these other uses do.  
Maybe we should just toss this code out, and try to find some other open 
source implementation of file name "globbing", and use it instead.

Comment 4

9 years ago
(In reply to comment #3)
> NSS's copy goes back to 1994. :)
> I got the reference to bug 498500 from these pages:
> Maybe those pages are lying to me about the bug that is relevant to their
> checkins (wouldn't surprise me).

Ugh, you misinterpreted the hgweb output completely, it just happens to show the latest commit that was made to the repository on top of the page, but that does have no connection if the shown file itself was even touched by this commit.
Click on the actual commit in left left column to see what changeset the affected line(s) was last changed in - in this case, the initial import of Mozilla source into hg, which has nothing to do with me ;-)

Comment 5

9 years ago
This is bug 332173, I think.
Thanks, Benjamin.  I concur.

Robert, Thanks for the explanation.  Sorry for my confusion.  I'm not (yet?) 
an Hg user.  I find this idea that every page for every source file cites 
the most recent commit, whether it had ANYTHING to do with this file or not, 
to be unintuitive and not very useful.  Is there ANY way to get a page that
lists just the changesets that actually affected the file being viewed?
Last Resolved: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 332173

Comment 7

9 years ago
Nelson, the "revisions" link, e.g. gives you all changesets that affected the file at all, in this case only the initial import into hg. In the annotate view you linked, the column of links in the left column (all "hg@1" in those links, i.e. done by the user "hg" in the first repository-wide revision) tells you which user/revision changed that specific line most recently. The revision and comment at the top tell you what revision of the repository you are looking at in general. The important concept here is that revisions/changesets are always for the whole repository and not per file as in CVS. With the revisions links you can look at revisions/changesets that affected a files though and with annotate at the last revision/changeset that affected a certain code line.
Thanks, Robert.
>  revisions/changesets are always for the whole repository and not
> per file as in CVS.
Yeah, that is precisely why Mozilla's Hg repositories are "downstream" 
repositories for NSS.  

BTW, I will shortly have a new patch for the old shexp.c code.  It's a 
pretty big change, but it's much better than what we have now, and must 
better than my previous patches to it.


9 years ago
blocking1.9.1: --- → .2+
status1.9.1: needstriage → wanted
Flags: blocking1.9.0.13?
Flags: blocking1.9.0.13+
Whiteboard: [Embargoed until at least 2009-07-29] → [sg:dupe 332173][Embargoed until at least 2009-07-29]
Fixed by bug 332173.
status1.9.1: wanted → .2-fixed
Nelson, could you help us verify this for 3.5.2?
I hereby verify that this is a duplicate of bug 332173.


9 years ago
Flags: blocking1.9.0.14+
Keywords: fixed1.9.0.13


9 years ago
Group: core-security


8 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.