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

Status: RESOLVED WONTFIX

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[[e-mail]]

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

[Jesse Ruderman]

Why is this in the "Rewriting and Analysis" component rather than "String"?

Comment 2

[[e-mail]]

Because I don't know yet if the code is actually used in an insecure manner.

Comment 3

[Jesse Ruderman]

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.

Comment 4

[[e-mail]]

The actual fix was in bug 479057.

Comment 5

[Jesse Ruderman]

Hmm, ok.  Any reason not to just land that patch on 1.9.1?

Comment 6

[[e-mail]]

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

Comment 7

[Jesse Ruderman]

Bug 479057 was fixed for 1.9.1.10, so this static analysis is no longer necessary.