Determine whether any part of Gecko was vulnerable to String bug 479057

RESOLVED WONTFIX

Status

()

Core
Rewriting and Analysis
RESOLVED WONTFIX
9 years ago
6 years ago

People

(Reporter: neil@parkwaycc.co.uk, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical] perhaps for 1.9.1.9 and older)

(Reporter)

Description

9 years ago
There appears to be a flaw in the internal string API for concatenation. The flaw is only exposed in a limited set of circumstances, so if the flaw is not exposed by our codebase we can just fix the bug on trunk and move on.

The flaw requires the following conditions:
1. The string that the concatenation is assigned to is also one of the arguments of the concatenation.
2. The string is not the first or last to be concatenated. (Although the flaw applies in this case, it simply overwrites the memory in-place with itself.)
3. The string is actually empty. (If it can be proved that the string is never empty, then the flaw does not apply.)

So for instance, take the following contrived example:
void ConvertHostToURL(nsCString& hostname) {
  hostname = 
    (StringBeginsWith(hostname, NS_LITERAL_CSTRING("ftp.")) ?
      NS_LITERAL_CSTRING("ftp://") :
      NS_LITERAL_CSTRING("http://")) + hostname + "/";
}
If the hostname string was empty, then the "http://" would actually get copied one (length of the "/") byte before the intended destination.

Comment 1

9 years ago
Why is this in the "Rewriting and Analysis" component rather than "String"?
(Reporter)

Comment 2

9 years ago
Because I don't know yet if the code is actually used in an insecure manner.
Whiteboard: [sg:investigate]

Updated

8 years ago
Assignee: tglek → nobody

Comment 3

8 years ago
Let's treat this as an sg:critical bug in the String component.  It's at least a hazard that needs to be fixed there.  Fixing it is probably much easier than determining whether any part of Gecko is vulnerable, and we have no idea about third-party code.
Component: Rewriting and Analysis → String
QA Contact: rewriting-and-analysis → string
Whiteboard: [sg:investigate] → [sg:critical]
(Reporter)

Comment 4

8 years ago
The actual fix was in bug 479057.

Comment 5

8 years ago
Hmm, ok.  Any reason not to just land that patch on 1.9.1?
Component: String → Rewriting and Analysis
QA Contact: string → rewriting-and-analysis
Summary: Possible to overwrite heap or stack memory by concatenating strings → Determine whether any part of Gecko was vulnerable to String bug 479057
Whiteboard: [sg:critical] → [sg:critical] perhaps for 1.9.1 branch and older
(Reporter)

Comment 6

8 years ago
Sure, you get me approval and I'll land it.

Comment 7

8 years ago
Bug 479057 was fixed for 1.9.1.10, so this static analysis is no longer necessary.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WONTFIX
Whiteboard: [sg:critical] perhaps for 1.9.1 branch and older → [sg:critical] perhaps for 1.9.1.9 and older
Group: core-security
You need to log in before you can comment on or make changes to this bug.