Note: There are a few cases of duplicates in user autocompletion which are being worked on.

XSLTProcessor.setParameter("","f3ck",array) causes heap trouble

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
XSLT
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: georgi - hopefully not receiving bugspam, Assigned: peterv)

Tracking

({verified1.8.0.7, verified1.8.1})

Trunk
mozilla1.9alpha1
verified1.8.0.7, verified1.8.1
Points:
---
Bug Flags:
blocking1.7.14 -
blocking-aviary1.0.9 -
blocking1.8.0.7 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] not in 1.7/aviary)

Attachments

(3 attachments)

XSLTProcessor.setParameter("","f3ck",array) causes heap trouble, 
probably either heap overflow or wild pointers.

on trunk, 2.0 and 1.5

*** glibc detected *** free(): invalid next size (fast): 0x08dd6ef8 ***
Created attachment 233448 [details]
this is a testcase - glibc whining
(Reporter)

Updated

11 years ago
Component: General → General
Product: Firefox → Core
(Reporter)

Updated

11 years ago
Component: General → Web Services
Assignee: nobody → xslt
Component: Web Services → XSLT
QA Contact: general → keith
(Assignee)

Comment 2

11 years ago
Created attachment 233495 [details] [diff] [review]
v1
Assignee: xslt → peterv
Status: NEW → ASSIGNED
Attachment #233495 - Flags: superreview?(bzbarsky)
Attachment #233495 - Flags: review?(bzbarsky)
(Assignee)

Comment 3

11 years ago
Created attachment 233496 [details] [diff] [review]
v1 (branch version)

Comment 4

11 years ago
Comment on attachment 233495 [details] [diff] [review]
v1

Yeah, makes sense.
Attachment #233495 - Flags: superreview?(bzbarsky)
Attachment #233495 - Flags: superreview+
Attachment #233495 - Flags: review?(bzbarsky)
Attachment #233495 - Flags: review+
(Assignee)

Comment 5

11 years ago
Georgi, could you verify whether this fixes the problems?
this seems fixed on today trunk.

if i understand correctly the side effects of macro arguments, NS_RELEASE doesn't seems misused in other places (according to grep)
(Assignee)

Comment 7

11 years ago
Comment on attachment 233496 [details] [diff] [review]
v1 (branch version)

NS_RELEASE first releases and then overwrites the pointer with 0. Since it evaluates the macro argument twice that means we could end up writing a null just past the end of the array.
Attachment #233496 - Flags: approval1.8.1?
Attachment #233496 - Flags: approval1.8.0.7?
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
OS: Linux → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
(In reply to comment #7)
> (From update of attachment 233496 [details] [diff] [review] [edit])
> NS_RELEASE first releases and then overwrites the pointer with 0. Since it
> evaluates the macro argument twice that means we could end up writing a null
> just past the end of the array.
> 

yes, that is what the macro definition is.

so it seems to allow writing zero past the last element.

and if the array.length > 1 it screws memory on the way to the end.

in comment #6 i meant that grep doesn't catch other misuses of NS_RELEASE(++/--)
Comment on attachment 233496 [details] [diff] [review]
v1 (branch version)

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword once you have done so.
Attachment #233496 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 233496 [details] [diff] [review]
v1 (branch version)

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #233496 - Flags: approval1.8.0.7? → approval1.8.0.7+
Flags: blocking1.8.1?
Flags: blocking1.8.0.7?
Whiteboard: [sg:critical?]
Flags: blocking1.8.0.7? → blocking1.8.0.7+
fix checked into 1.8 and 1.8.0 branches
Keywords: fixed1.8.0.7, fixed1.8.1

Comment 13

11 years ago
v.fixed on both branches by code inspection, this was a simple fix.  If anyone has a testcase, we can retest.
Keywords: fixed1.8.0.7, fixed1.8.1 → verified1.8.0.7, verified1.8.1
Whiteboard: [sg:critical?] → [sg:critical?][need testcase]
(In reply to comment #13)
> v.fixed on both branches by code inspection, this was a simple fix.  If anyone
> has a testcase, we can retest.
> 

the testcase is in this bug titled "glibc whining"

Comment 15

11 years ago
the affected code appears not to exist on 1.0.x ... Peter, can you confirm that aviary 1.0.1 branch is not affected?
(Assignee)

Comment 16

11 years ago
(In reply to comment #15)
> the affected code appears not to exist on 1.0.x ... Peter, can you confirm that
> aviary 1.0.1 branch is not affected?

It is not affected.
Flags: blocking1.7.14-
Flags: blocking-aviary1.0.9-
Whiteboard: [sg:critical?][need testcase] → [sg:critical?][need testcase] not in 1.7/aviary
(Reporter)

Updated

11 years ago
Attachment #233448 - Attachment description: glibc whining → this is a testcase - glibc whining
as per comment #14 there is testcase attached in this bug.

just renamed the testcase, removing "need testcase"
Whiteboard: [sg:critical?][need testcase] not in 1.7/aviary → [sg:critical?] not in 1.7/aviary
Group: security
Flags: in-testsuite?
Flags: blocking1.8.1?
You need to log in before you can comment on or make changes to this bug.