The default bug view has changed. See this FAQ.

Firefox 1.0.6 buffer overflow with hostname of all soft hyphens [@ nsStringBuffer::Realloc] [@ nsCSubstring::Capacity] [@ nsGenericElement::~nsGenericElement]

RESOLVED FIXED in mozilla1.8beta5

Status

()

Core
Networking
P1
critical
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Tom Ferris, Assigned: dbaron)

Tracking

(5 keywords)

Trunk
mozilla1.8beta5
crash, fixed-aviary1.0.7, fixed1.7.12, testcase, verified1.8
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.7.12 +
blocking-aviary1.0.7 +
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix], crash signature, URL)

Attachments

(8 attachments, 7 obsolete attachments)

23.23 KB, text/plain
Details
61 bytes, text/html
Details
3.36 KB, patch
Darin Fisher
: review+
dveditz
: superreview+
Scott MacGregor
: approval1.8b5+
Details | Diff | Splinter Review
3.92 KB, patch
Darin Fisher
: review+
dveditz
: superreview+
Details | Diff | Splinter Review
564 bytes, application/octet-stream
Details
632 bytes, application/x-xpinstall
Details
24.02 KB, patch
Darin Fisher
: review+
dveditz
: superreview+
Details | Diff | Splinter Review
24.62 KB, text/html
Details
(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050416 Epiphany/1.2.8
Build Identifier: 

I have found a format string vulnerability within Firefox 1.0.6 which allows for
arbitrary code execution.

Affect Versions:

Firefox Linux 1.0.6
Firefox Win32 1.0.6
Deer Park Latest Build

Below is a url to reproduce this issue:

http://www.security-protocols.com/firefoxwin32-death.html

If you need any further information, please let me know.

Thanks,

Tom Ferris
Researcher
www.security-protocols.com
Key fingerprint = 0DFA 6275 BA05 0380 DD91  34AD C909 A338 D1AF 5D78

Reproducible: Always

Steps to Reproduce:

Updated

12 years ago
Blocks: 264944

Comment 1

12 years ago
Created attachment 195049 [details]
apple crash report (Gecko 1.8 branch)

Corresponds to TB9093897Y.

Updated

12 years ago
Hardware: PC → All
Summary: Firefox 1.0.6 Format String → Firefox 1.0.6 Format String [@ nsStringBuffer::Realloc]

Updated

12 years ago
Summary: Firefox 1.0.6 Format String [@ nsStringBuffer::Realloc] → Firefox 1.0.6 Format String [@ nsStringBuffer::Realloc] [@ nsCSubstring::Capacity]

Updated

12 years ago
Summary: Firefox 1.0.6 Format String [@ nsStringBuffer::Realloc] [@ nsCSubstring::Capacity] → Firefox 1.0.6 Format String [@ nsStringBuffer::Realloc] [@ nsCSubstring::Capacity] [@ nsGenericElement::~nsGenericElement]
(Assignee)

Comment 2

12 years ago
So the reason I crash here on Linux is not a format string vulnerability; it's a
buffer overflow.

In particular, the presence of a hostname of all dashes (and a rather long one
at that) causes the NormalizeIDN call in nsStandardURL::BuildNormalizedSpec to
return true but set encHost to the empty string.  This means we append 0 to
approxLen, but later on append the long string of dashes to the buffer instead.
Status: UNCONFIRMED → NEW
Component: General → Networking
Ever confirmed: true
Product: Firefox → Core
Summary: Firefox 1.0.6 Format String [@ nsStringBuffer::Realloc] [@ nsCSubstring::Capacity] [@ nsGenericElement::~nsGenericElement] → Firefox 1.0.6 buffer overflow with hostname of all dashes [@ nsStringBuffer::Realloc] [@ nsCSubstring::Capacity] [@ nsGenericElement::~nsGenericElement]
Version: unspecified → Trunk

Comment 3

12 years ago
Created attachment 195056 [details]
partially simplified testcase (crashes firefox on gecko 1.8 branch)
(Assignee)

Comment 4

12 years ago
Created attachment 195062 [details] [diff] [review]
ugly patch to fix trunk crash

This fixes the trunk crash for me on Linux.  I'm not sure if it's the right
fix, though.  It may be that a hostname of all dashes should be considered
invalid ACE or whatever it is that makes NormalizeIDN think it can decode it,
in which case NormalizeIDN should return false in this case.  And it might be
safer to do that in either case.  I'm still a bit puzzled by some other things
I saw with the testcase, though -- in particular, that it seemed like an
embedded null got into the string somehow.

Comment 5

12 years ago
Created attachment 195063 [details]
Simplified testcase (crashes Firefox on Gecko 1.8 branch and trunk)

This may or may not be the same crash dbaron was working on.
Attachment #195056 - Attachment is obsolete: true
On windows I get completely different stacks.

On 1.0.6 I crash in js_SetProtoOrParent: TB9095338

In Deer Park I crash in operator new, handling a mouse event? TB9094980. The
only mouse event I can think of is the one clicking on the testcase link, but it
seemed to crash quite some time after that.

I'll try with the patch and see.

Comment 7

12 years ago
The testcase in comment 5 doesn't crash when I load it from Bugzilla, but it
does crash if I save it to my desktop and load it from there.

Updated

12 years ago
Keywords: crash
Whiteboard: [sg:fix]
(Assignee)

Comment 8

12 years ago
The net_ToLowerCase call is also a potential buffer overrun here.
(Assignee)

Comment 9

12 years ago
The reason I was confused about the lengths is that it was a long string of soft
hyphens (U+00AD), encoded in UTF-8 as (0xC2 0xAD).  So it wasn't quite the
normalization bug I thought it was, but there's definitely still some sort of
bug here.
(Assignee)

Updated

12 years ago
Summary: Firefox 1.0.6 buffer overflow with hostname of all dashes [@ nsStringBuffer::Realloc] [@ nsCSubstring::Capacity] [@ nsGenericElement::~nsGenericElement] → Firefox 1.0.6 buffer overflow with hostname of all soft hyphens [@ nsStringBuffer::Realloc] [@ nsCSubstring::Capacity] [@ nsGenericElement::~nsGenericElement]
With the patch I still crash on both trunk and branch with the original
testcase, unfortunately in all kinds of different places on corrupted or freed
objects making it very hard to tell what's gone wrong.

Comment 11

12 years ago
The host-all-hyphens crash is now public:
http://www.security-protocols.com/modules.php?name=News&file=article&sid=2910

I'm not making this bug public immediately because of dveditz' comment 10.
Flags: blocking1.8b5?
Flags: blocking-aviary1.0.7?
(Assignee)

Comment 12

12 years ago
Also http://seclists.org/lists/fulldisclosure/2005/Sep/index.html#233
(Assignee)

Comment 13

12 years ago
FWIW, the IDN RFCs are:
ftp://ftp.rfc-editor.org/in-notes/rfc3490.txt
ftp://ftp.rfc-editor.org/in-notes/rfc3491.txt
ftp://ftp.rfc-editor.org/in-notes/rfc3492.txt
and stringprep is:
ftp://ftp.rfc-editor.org/in-notes/rfc3454.txt

Comment 14

12 years ago
With network.enableIDN=true this reproduces easily for me.  With
network.enableIDN=false I do not crash.

Comment 15

12 years ago
Can anyone verify that this issue ONLY works if the hostname is all -'s?  While
this will still be a crash, it won't allow arbitrary code execution.
(Assignee)

Comment 16

12 years ago
Wrong, since there's stuff appended to the buffer after the hostname.
(Assignee)

Comment 17

12 years ago
*** Bug 307722 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 18

12 years ago
*** Bug 307725 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Flags: blocking1.8b5?
Flags: blocking1.8b5+
Flags: blocking1.7.12+
Flags: blocking-aviary1.0.7?
Flags: blocking-aviary1.0.7+
(Assignee)

Comment 19

12 years ago
Created attachment 195421 [details] [diff] [review]
corrected hack fix 

attachment 195062 [details] [diff] [review] wasn't sufficient, as I pointed out in comment 8, and
actually had other bugs as well.  This ought to be, although it's still not the
right long term fix.  dveditz, could you retest with this one?
Attachment #195062 - Attachment is obsolete: true
(Assignee)

Comment 20

12 years ago
Created attachment 195422 [details] [diff] [review]
aviary branch version of hack fix
(Assignee)

Comment 21

12 years ago
Comment on attachment 195421 [details] [diff] [review]
corrected hack fix 

>+            AppendToBuf(buf, i, encHost.get(), mHost.mLen);
>+            i += mHost.mLen;

This can be simplified in both patches to:

+	     i = AppendToBuf(buf, i, encHost.get(), mHost.mLen);
(Assignee)

Comment 22

12 years ago
Created attachment 195424 [details] [diff] [review]
hack fix
Attachment #195421 - Attachment is obsolete: true
Attachment #195422 - Attachment is obsolete: true
Attachment #195424 - Flags: superreview?(dveditz)
Attachment #195424 - Flags: review?(darin)
(Assignee)

Comment 23

12 years ago
Created attachment 195425 [details] [diff] [review]
aviary branch version of hack fix

Comment 24

12 years ago
This issue has been assigned the CVE id CAN-2005-2871.
Comment on attachment 195424 [details] [diff] [review]
hack fix

sr=dveditz
This version fixes the remaining memory corruption I was seeing.
Attachment #195424 - Flags: superreview?(dveditz) → superreview+
Blocks: 256195
(Assignee)

Comment 26

12 years ago
OK, removing security-confidential flag since everything in this bug is public.
Group: security
Blocks: 307752

Comment 27

12 years ago
Can we confirm whether enableIDN=false fixes the problem?

Comment 28

12 years ago
Comment on attachment 195424 [details] [diff] [review]
hack fix

r=darin

nit: insert newline between closing bracket and "else"
Attachment #195424 - Flags: review?(darin) → review+

Updated

12 years ago
Attachment #195425 - Flags: review+
(Assignee)

Updated

12 years ago
Attachment #195424 - Flags: approval1.8b5?
(Assignee)

Updated

12 years ago
Attachment #195425 - Flags: approval1.7.12?
Attachment #195425 - Flags: approval-aviary1.0.7?
(Assignee)

Comment 29

12 years ago
Looking at the code on the trunk, the "network.enableIDN" pref being false
should always avoid the problematic codepath since gIDN will be null.
(Assignee)

Comment 30

12 years ago
Comment on attachment 195424 [details] [diff] [review]
hack fix

Checked in to trunk, 2005-09-09 12:06 -0700.

Comment 31

12 years ago
Would setting network.enableIDN to false be a viable work-around until the bug
is fully fixed?
If so, given the high rating Secunia gives this, should we be advising people to
make that change?

Comment 32

12 years ago
I've tested 1.0.6 with Windows XP, Windows 2000, and Windows 98 and disabling
IDN avoids the crash in the URL testcase.  We should get a XPI out to toggle
this pref.

Comment 33

12 years ago
Yes, you should be able to set enableIDN to false in about:config to workaround
this bug.

Comment 34

12 years ago
Created attachment 195447 [details]
XPI to disable IDN and bump the vender Sub to 1.0.6.1

This XPI installs a new file in the application default prefs directory that
adds the following default preferences:

pref("app.version", "1.0.6.1");
pref("general.useragent.vendorSub", "1.0.6.1");
pref("network.enableIDN", false);

Comment 35

12 years ago
We are confirming for sure the enableIDN=false fixes it and if so will be
posting the workaround shortly.

Comment 36

12 years ago
We are preparing the announcement to http://www.mozilla.org/security/.  Can
people here assist in verification of the XPI:
   a) Does it set enableIDN=false
   b) Does it bump the about dialog version to 1.0.6.1
   c) Does it prevent the crash as described here.

Thanks!

Mike

Comment 37

12 years ago
Created attachment 195452 [details]
untested seamonkey XPI bumps the version to 1.7.11.1

Comment 38

12 years ago
(In reply to comment #36)
> We are preparing the announcement to http://www.mozilla.org/security/.  Can
> people here assist in verification of the XPI:
>    a) Does it set enableIDN=false
>    b) Does it bump the about dialog version to 1.0.6.1
>    c) Does it prevent the crash as described here.
> 
> Thanks!
> 
> Mike
> 

a) yes
b) yes
c) yes
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-GB; rv:1.7.10) Gecko/20050717
Firefox/1.0.6.1
> 

Comment 39

12 years ago
I can verify that Scott's xpi prevents the crash from the simplified testcase
(when opening the crash.html locally).  As Jessa mentioned, the testcase doesn't
crash (with the default prefs) if you try to open it from this bug, but it when
saved locally.

I tested it with Firefox 1.0.6 on Windows XP and Linux (FC 4).

Comment 40

12 years ago
XPI patches verified on both Firefox and Suite.

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.10) Gecko/20050716
Firefox/1.0.6.1
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; ja-JP; rv:1.7.11) Gecko/20050727

Comment 41

12 years ago
With Firefox 1.5b1 two of the prefs no longer exist:
app.version
general.useragent.vendorSub

Therefore applying a the xpi patch DOES properly set pref("network.enableIDN",
false);, but adds and sets the other 2 version related prefs (which are no
longer used), so the version bump doesn't happen as expected. 

If we plan to patch Beta 1, we'll have to figure out what the new version prefs
are and create a separate patch.

Updated

12 years ago
Keywords: testcase

Comment 42

12 years ago
Created attachment 195467 [details]
new xpi for seamonkey and firefox, sets vendorComment instead of changing the version

Here's an updated XPI file which doesn't change the version string at all.

Instead if just adds a No IDN vendor comment. This allows us to use the same
XPI for the suite and for any 1.0.x version of Firefox and the 1.5 beta.
Attachment #195447 - Attachment is obsolete: true
Attachment #195452 - Attachment is obsolete: true

Comment 43

12 years ago
XPI patch v2 looks good. But its comment should be more descriptive.

- pref("general.useragent.vendorComment", "No IDN");
+ pref("general.useragent.vendorComment", "IDN support is disabled for security
reasons. See Bug 307259 for details.");

Comment 44

12 years ago
Sorry, "general.useragent.vendorComment" should be a few simple words.
Ignore my comment above.

Comment 45

12 years ago
New patch works for me on 1.0.6
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-GB; rv:1.7.10) Gecko/20050717
Firefox/1.0.6

However on 1.5 Beta 1 it does not seem to show the vendor comment in Help >
About, although the modified preference is correct in about:config
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

Also, if you have the ActiveX plugin installed, that sets a vendor comment of
(ax) which (for me) overrode the No IDN vendor commment in 1.0.6

Comment 46

12 years ago
P.S. on Deer Park it did still successfully disable IDN and so did apply the
workaround (it survuded the testcase) - I just didn't get the vendor comment

Comment 47

12 years ago
Created attachment 195471 [details]
update xpi to set productComment

This updated XPI sets the product comment instead of the vendor comment. 

The vendor comment did not show up on mozilla suite and firefox 1.5b1 because
those  products don't have vendorSub strings. By moving to the productComment
we think the  No IDN text will show up on all 3 of those builds.
Attachment #195467 - Attachment is obsolete: true

Comment 48

12 years ago
The latest XPI works for me on FF 1.0.6 / WinXP: enableIDN is properly set to
false, the text "(No IDN)" appears after "Gecko/20050716" in the UA string, and
the crash is gone.
So we're going to try using the general.useragent.productComment pref. Not
ideal because some UA sniffers might not be expecting a comment string between
the Gecko and Firefox tokens (they are more tolerant of end garbage), but will
work on all Gecko-based browsers. Keep in mind that we will be issuing an actual
patched build in the near future, and then this whole comment thing goes away.

Comment 50

12 years ago
Survives testcase and adds No IDN product comment in my installs of both FF
1.0.6 and FF 1.5 Beta 1

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-GB; rv:1.7.10) Gecko/20050717 (No
IDN) Firefox/1.0.6 (ax)
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 (No
IDN) Firefox/1.4

Comment 51

12 years ago
I have verified the patch from comment #47 works for:

FF106: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716
(No IDN) Firefox/1.0.6
FF15b1: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908
(No IDN) Firefox/1.4
M1711: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.11) Gecko/20050728
(No IDN)

All three have their network.enableIDN=false and are crash free.
Looks good on Mac, both FF 1.0.6 and FF 1.5b1. The No IDN text appears in the
about firefox and the enable IDN is set to false.
and looks good in the Mozilla suite as well, forgot to add that - Mozilla 1.7.11
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.11) Gecko/20050727
(No IDN)

(In reply to comment #52)
> Looks good on Mac, both FF 1.0.6 and FF 1.5b1. The No IDN text appears in the
> about firefox and the enable IDN is set to false.
No longer blocks: 307752

Comment 54

12 years ago
Comment on attachment 195424 [details] [diff] [review]
hack fix

approving for 1.8b5
Attachment #195424 - Flags: approval1.8b5? → approval1.8b5+
Comment on attachment 195425 [details] [diff] [review]
aviary branch version of hack fix

sr=dveditz
a=dveditz for aviary and 1.7 branches
Attachment #195425 - Flags: superreview+
Attachment #195425 - Flags: approval1.7.12?
Attachment #195425 - Flags: approval1.7.12+
Attachment #195425 - Flags: approval-aviary1.0.7?
Attachment #195425 - Flags: approval-aviary1.0.7+
*** Bug 307800 has been marked as a duplicate of this bug. ***

Comment 57

12 years ago
Firefox 1.5 beta 1 should be patched via the new app update system.  There's no
reason to cut an XPI for it.  For the update to 1.5 beta 1 (is it going to be
1.4.1 or 1.4.0.1?), we should just take dbaron's patch and not futz with the
enableIDN pref.
(In reply to comment #57)
> Firefox 1.5 beta 1 should be patched via the new app update system.  There's no
> reason to cut an XPI for it.  For the update to 1.5 beta 1 (is it going to be
> 1.4.1 or 1.4.0.1?), we should just take dbaron's patch and not futz with the
> enableIDN pref.

1.5 should and most likely will be patched with the update system (what a great
opportunity to test it!), but we're not going to build/test/push over the
weekend. For users who see the press over the weekend it's nice to have an easy
patch for them. We'll make sure the next version's installers removes the pref file.

I know the patch system can remove files... will it fail if the file is not
there to remove? that is, some number of users will have the file to be removed,
but most will not. If removing a non-existing file causes problems then it's
better not to mess with it.

Comment 59

12 years ago
This is SA16764 at http://secunia.com/advisories/16764/ and FrSIRT/ADV-2005-1690
at http://www.frsirt.com/english/advisories/2005/1690 (both rated as Critical).
Is there a change to update Alias field to SA16764 etc.

Comment 60

12 years ago
Information about new http://www.mozilla.org/security/idn.html page spreaded
with Internet Storm Center, MozillaZine etc.
(Assignee)

Comment 61

12 years ago
attachment 195424 [details] [diff] [review] checked in to trunk, 2005-09-09 12:06 -0700.
attachment 195425 [details] [diff] [review] checked in to AVIARY_1_0_1_20050124_BRANCH, 2005-09-09 16:33
-0700.
attachment 195425 [details] [diff] [review] checked in to MOZILLA_1_7_BRANCH, 2005-09-09 16:34 -0700.
attachment 195424 [details] [diff] [review] checked in to MOZILLA_1_8_BRANCH, 2005-09-10 00:12 -0700.
Assignee: nobody → dbaron
Keywords: fixed1.8
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta5
(Assignee)

Updated

12 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 62

12 years ago
[10 12:33:56] <rob_strong> JAY: looks like the 307259.xpi was packaged with
incorrect perms for Linux
...
[10 12:39:45] <dbaron> rob_strong, do you mean that the permissions inside the
XPI are wrong?
[10 12:41:39] <rob_strong> dbaron: yes
[10 12:42:16] <rob_strong> dbaron: same as happened last time iirc
[10 12:42:54] <dbaron> what are they, and what should they be?
[10 12:43:06] <rob_strong> they are  400
[10 12:43:29] <rob_strong> They should be +r for all
[10 12:45:05] <rob_strong> Here is a handy py script for verifying
http://biomikro.vscht.cz/maldiman/hassmanm/projects/testzip.py.txt
[10 12:51:43] <dbaron> mind if I paste this in the bug?
[10 12:52:26] <rob_strong> Not at all
Created attachment 195573 [details]
updated xpi with file perms 0644

for what it is worth the files in this xpi are the same as on the mirrors but
have been packaged with perms set to 0644.
(Assignee)

Comment 64

12 years ago
Created attachment 195682 [details] [diff] [review]
belt-and-braces patch

Rather than figure out whether we really need this, I propose we take this as
well.

I also changed two function names that are dangerously unclear.
Attachment #195682 - Flags: superreview?(dveditz)
Attachment #195682 - Flags: review?(darin)
(Assignee)

Comment 65

12 years ago
The SetQuery and SetRef changes in that patch aren't quite right, though...
(Assignee)

Comment 66

12 years ago
Actually, I think they're OK, since they match what BuildNormalizedSpec does.
(Assignee)

Updated

12 years ago
Flags: blocking1.7.13+
Flags: blocking1.7.12+
Flags: blocking-aviary1.0.8+
Flags: blocking-aviary1.0.7+
(Assignee)

Updated

12 years ago
Attachment #195682 - Flags: approval1.8b5?
Attachment #195682 - Flags: approval1.7.12?
Attachment #195682 - Flags: approval-aviary1.0.7?
Comment on attachment 195682 [details] [diff] [review]
belt-and-braces patch

sr=dveditz
Attachment #195682 - Flags: superreview?(dveditz) → superreview+

Comment 68

12 years ago
Does the belt-and-braces patch contribute to fix this specific problem or is
this  just a preventive measure like the name 'belt-and-braces' suggests? 

If so, I would rather suggest to not send this patch to the aviary branch. Would
be great to keep the changeset minimal for the stable branch.
It appears that the 307259.xpi on the mirrors is still the one with bad perms
for unix... :/

Comment 70

12 years ago
Comment on attachment 195682 [details] [diff] [review]
belt-and-braces patch

It almost seems to me that a small helper class that combines a
string object with a boolean flag would be nice here.


>Index: nsStandardURL.cpp

>+    EncodeSegmentCount(str.BeginReading(text), URLSegment(0, str.Length()), mask, result, encoded);
>+    if (encoded)
>         return result;
>     else
>         return str;
> }

I know this isn't your code, but if you would, please change that to this:

   return encoded ? result : str;

Or, at least remove the unnecessary 'else'.  thanks!


r=darin
Attachment #195682 - Flags: review?(darin) → review+

Comment 71

12 years ago
Will this bug alone cause a 1.0.7 release? Or will there be a waiting period in
case there are more? I see it is marked blocking, but how urgent is it? Thanks
for the quick fix.
(In reply to comment #69)
> It appears that the 307259.xpi on the mirrors is still the one with bad perms
> for unix... :/

ftp-chi.osuosl.org was the odd-one-out when I checked all the md5sums.  Everyone
else had the correct version.  I contacted the admin, and it's probably been
fixed already by the time I'm typing this.
(In reply to comment #72)
> ftp-chi.osuosl.org was the odd-one-out when I checked all the md5sums.  Everyone
> else had the correct version.  I contacted the admin, and it's probably been
> fixed already by the time I'm typing this.
Are you sure? I checked on a couple of the mirrors and they aren't fixed for me.
I also checked using the python script (
http://biomikro.vscht.cz/maldiman/hassmanm/projects/testzip.py.txt ) which shows
they are packaged with 400 perms and the dates are from the 9th which is before
the permissions issue was brought up on 20050910.
http://mozilla.ussg.indiana.edu/pub/mozilla.org/firefox/releases/1.0.6/patches/
http://149.174.36.116/pub/mozilla.org/firefox/releases/1.0.6/patches/

Updated

12 years ago
Attachment #195682 - Flags: approval1.8b5?
Attachment #195682 - Flags: approval1.8b5+
Attachment #195682 - Flags: approval1.7.12?
Attachment #195682 - Flags: approval1.7.12+
Attachment #195682 - Flags: approval-aviary1.0.7?
Attachment #195682 - Flags: approval-aviary1.0.7+
(Assignee)

Comment 74

12 years ago
attachment 195682 [details] [diff] [review] checked in:
 * to trunk, 2005-09-13 10:38 -0700
 * to MOZILLA_1_8_BRANCH, 2005-09-13 11:23 -0700
attachment 195682 [details] [diff] [review] excluding the renames of functions (which didn't exist on the
old branches) checked in:
 * to AVIARY_1_0_1_20050124_BRANCH, 2005-09-13 11:23 -0700
 * to MOZILLA_1_7_BRANCH, 2005-09-13 11:24 -0700
Keywords: fixed-aviary1.0.7, fixed1.7.12
(In reply to comment #73)
> (In reply to comment #72)
> > ftp-chi.osuosl.org was the odd-one-out when I checked all the md5sums.
> > Everyone else had the correct version.  I contacted the admin, and it's
> > probably been fixed already by the time I'm typing this.
> Are you sure? I checked on a couple of the mirrors and they aren't fixed for
> me.

All 17 servers match md5sums now, and all have the same md5sum as stage.  If
it's wrong, then they're all wrong, and the correct xpi apparently hasn't been
uploaded yet.
Depends on: 307899
Correct... at least now the xpi with the incorrect file permissions for UNIX is
properly distributed to all the mirrors. :P

I verified using the py script and by installing the patch that it does indeed
have no access set for everyone except root when installed as root so the patch
is not applied to anyone but the root account (e.g. no access means it isn't
read by the pref system since it can't access it). See bug 189905 for details as
to the underlying details.

Comment 77

12 years ago
Well that's not good at all. dbaron: if you can take a quick look at this and
make sure that Rob's update works as expected, I can push it to the ftp site. Do
we need to go further with this, or just post the xpi with correct permissions
and push on to 1.0.7?

Comment 78

12 years ago
did someone actually push the xpi with permission bits set for linux over the
weekend or did the xpi just get posted to this bug? I'm more than happy to
upload the new version. I thought someone had already done so. 

Comment 79

12 years ago
According to stage, the xpi hasn't been modified since September 9th (Rob's
patch was posted on the 10th for those following along at home). This seems like
another one of those "someone will get it" situations... Scott: are you going to
post the xpi, or should I go ahead and do it? 
If there is anything I can do to help just ask... also, it might not be such a
bad idea to run the python script on any future xpi based patches to verify the
perms. This is the second time this has happened out of the two or three times
xpi patches have been pushed in the last year.

Comment 81

12 years ago
(In reply to comment #77)
> Well that's not good at all. dbaron: if you can take a quick look at this and
> make sure that Rob's update works as expected, I can push it to the ftp site. Do
> we need to go further with this, or just post the xpi with correct permissions
> and push on to 1.0.7?

I've pushed dbaron's updated XPI to the FTP site.  Here are the MD5 checksums
for the 307259.xpi file:

Old 307259.xpi: d3d1c4d2dac9b90143574ca7e2fa8330
New 307259.xpi: c58d917c5dab7bbd18ec9807485cb7d4
(Reporter)

Comment 82

12 years ago
FYI:

http://security-protocols.com/modules.php?name=News&file=article&sid=2920
(Reporter)

Comment 83

12 years ago
FYI:

http://security-protocols.com/modules.php?name=News&file=article&sid=2920(In
reply to comment #82)
> FYI:
> 
> http://security-protocols.com/modules.php?name=News&file=article&sid=2920

almost forget the PoC:

http://www.security-protocols.com/deerpark-death.html
(In reply to comment #83)
> FYI:
> 
> http://security-protocols.com/modules.php?name=News&file=article&sid=2920(In
> reply to comment #82)
> > FYI:
> > 
> > http://security-protocols.com/modules.php?name=News&file=article&sid=2920
> 
> almost forget the PoC:
> 
> http://www.security-protocols.com/deerpark-death.html

What does that crash bug (in 1.5b1 only) have to do with this bug?

/be
(In reply to comment #82)
> FYI:
> 
> http://security-protocols.com/modules.php?name=News&file=article&sid=2920

Filed bug 308579 on the new crash. It's a completely unrelated null dereference
despite the superficial similarity in the testcase.

(In reply to comment #85)
> (In reply to comment #82)
> > FYI:
> > 
> > http://security-protocols.com/modules.php?name=News&file=article&sid=2920
> 
> Filed bug 308579 on the new crash. It's a completely unrelated null dereference
> despite the superficial similarity in the testcase.

Tom Ferris: your public statements on this matter of fact are incorrect and
misleading.  What are you going to do about that?

/be

Comment 87

12 years ago
ff 1.0.6/winxp: URL->crash, simplified->no crash, firefoxwin32-death->crash
ff 1.0.7/winxp; ff 1.5/winxp (20050914) no crash.
Created attachment 197418 [details]
Original testcase from Ferris

The contents at the testcase link in comment 0 have been changed to Jesse's
simplified testcase from comment 5. Just for the record this is the original
testcase from Tom Ferris.
Blocks: 256197
No longer blocks: 256195

Comment 89

12 years ago
no crash firefox 1.5 rc2 winxp/linux
Keywords: fixed1.8 → verified1.8
(Assignee)

Comment 90

11 years ago
Comment on attachment 195425 [details] [diff] [review]
aviary branch version of hack fix

Clearing aviary1.0.8/1.7.13 approval flags on this attachment because attachment 195682 [details] [diff] [review] is already in.
Attachment #195425 - Flags: approval1.7.13+
Attachment #195425 - Flags: approval-aviary1.0.8+

Comment 91

8 years ago
Are crashtests appropriate Is or wanted for this? Would they live in netwerk/base/src/crashtests ?
Crash Signature: [@ nsStringBuffer::Realloc] [@ nsCSubstring::Capacity] [@ nsGenericElement::~nsGenericElement]
You need to log in before you can comment on or make changes to this bug.