strndup implementation in memory/build/mozmemory_wrap.c is broken

RESOLVED FIXED in Firefox 20

Status

()

Core
Memory Allocator
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla20
Points:
---

Firefox Tracking Flags

(firefox17 affected, firefox18 affected, firefox19 affected, firefox20 fixed, firefox-esr10 affected, firefox-esr17 affected, b2g18 affected, b2g18-v1.0.1 affected)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
That strndup definition, inherited from bug 559263, doesn't handle a partial copy of another string properly.
(Assignee)

Updated

5 years ago
Summary: strndup implementation in memory/build/mozmemory_wrap.h is broken → strndup implementation in memory/build/mozmemory_wrap.c is broken
(Assignee)

Comment 1

5 years ago
Created attachment 697358 [details] [diff] [review]
Fix strndup in memory/build/mozmemory_wrap.c
Attachment #697358 - Flags: review?(justin.lebar+bug)
(Assignee)

Updated

5 years ago
status-b2g18: --- → affected
status-firefox-esr10: --- → affected
status-firefox17: --- → affected
status-firefox18: --- → affected
status-firefox19: --- → affected
status-firefox20: --- → affected
status-firefox-esr17: --- → affected
Comment on attachment 697358 [details] [diff] [review]
Fix strndup in memory/build/mozmemory_wrap.c

o.O
Attachment #697358 - Flags: review?(justin.lebar+bug) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/facbbe435906

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/facbbe435906
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20

Updated

5 years ago
status-firefox20: affected → fixed
Batch edit: Bugs marked status-b2g18: affected after 2/13 branching of v1.0.1 are now also status-b2g18-v1.0.1: affected
status-b2g18-v1.0.1: --- → affected

Comment 6

5 years ago
This sounds like the kind of bug that should get fixed everywhere. Should we request approval on the patch for most branches?

Comment 7

5 years ago
For reference, this affects modules loaded into a firefox process, which expect the libc implementation:

https://bugzilla.redhat.com/show_bug.cgi?id=922904
(In reply to Kai Engert (:kaie) from comment #6)
> This sounds like the kind of bug that should get fixed everywhere. Should we
> request approval on the patch for most branches?

I think we didn't take it on branches because we had no reason to believe that this fix had any effect on users, and that's a criterion that's usually applied to uplift requests.

If that belief is not true, I'd certainly be OK uplifting this to branches.

Comment 9

5 years ago
(In reply to Justin Lebar [:jlebar] from comment #8)
> (In reply to Kai Engert (:kaie) from comment #6)
> > This sounds like the kind of bug that should get fixed everywhere. Should we
> > request approval on the patch for most branches?
> 
> I think we didn't take it on branches because we had no reason to believe
> that this fix had any effect on users, and that's a criterion that's usually
> applied to uplift requests.
> 
> If that belief is not true, I'd certainly be OK uplifting this to branches.

Firefox exports these functions into the global symbol namespace. I'm not sure what the reasoning is for doing that. 

Because these functions override glibc functions, bugs in them have effects on any module loaded by firefox (imagine NPAPI plugin, PKCS#11 module, or binary add-on) making use of the given function. In this case the bug was seen when using the p11-kit-trust.so PKCS#11 module:

https://bugzilla.redhat.com/show_bug.cgi?id=922904

Comment 10

5 years ago
Comment on attachment 697358 [details] [diff] [review]
Fix strndup in memory/build/mozmemory_wrap.c

Proposing to fix on all active branches, in order to prevent random misbehaviour.
Attachment #697358 - Flags: approval-mozilla-esr17?
Attachment #697358 - Flags: approval-mozilla-beta?
Attachment #697358 - Flags: approval-mozilla-aurora?

Updated

5 years ago
Target Milestone: mozilla20 → mozilla22

Comment 11

5 years ago
It's already in 20.0 Beta and 21.0a2.
Target Milestone: mozilla22 → mozilla20
(Assignee)

Comment 12

5 years ago
Comment on attachment 697358 [details] [diff] [review]
Fix strndup in memory/build/mozmemory_wrap.c

It's already on aurora, beta, and doesn't affect esr17 on linux, I'm pretty sure it doesn't affect esr17 on mac, and it probably doesn't affect esr17 on windows.
Attachment #697358 - Flags: approval-mozilla-esr17?
Attachment #697358 - Flags: approval-mozilla-beta?
Attachment #697358 - Flags: approval-mozilla-aurora?
You need to log in before you can comment on or make changes to this bug.