367743 - Potential memory leak in AddCharsToURI (jsstr.c)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gavin Reaney]

If the following realloc fails, 'str-chars' will be leaked in AddCharsToURI:

str->chars = JS_realloc(cx, str->chars, total * sizeof(jschar));

Comment 1

[Gavin Reaney]

Attachment: Don't leak str->chars if realloc fails

Brian, would you mind reviewing this?

Comment 2

[Gavin Reaney]

(In reply to comment #1)
> Created an attachment (id=252333) [details]
> Don't leak str->chars if realloc fails
> 
> Brian, would you mind reviewing this?

Try again with updated cc-list.

Comment 3

[Jeff Walden [:Waldo]]

> > Brian, would you mind reviewing this?
> 
> Try again with updated cc-list.

Separate email is sent for review requests beyond even regular bugmail, and at least in my quick scan of email prefs I see no way to turn it off (and it seems highly unlikely such a preference would ever be turned off by a patch-reviewing developer).  Just a note for the future.  :-)

Comment 4

[Brian Crowder]

Comment on attachment 252333 [details] [diff] [review]
Don't leak str->chars if realloc fails

This looks good.  Did you, by any chance, wander around and audit other reallocs in the spidermonkey code for other bugs in this style?  I think jsregexp.c has one around line 650, and again on 789, 3389.  Should probably be another bug.

Comment 5

[Gavin Reaney]

(In reply to comment #4)
> (From update of attachment 252333 [details] [diff] [review])
> This looks good.  Did you, by any chance, wander around and audit other
> reallocs in the spidermonkey code for other bugs in this style?  I think
> jsregexp.c has one around line 650, and again on 789, 3389.  Should probably be
> another bug.
> 

Good point. I think the jsregexp realloc at 3389 is OK (the old pointer is retained by res->moreParens), but I agree that the other two should be fixed.

If anyone has a chance to land this patch on the trunk that would be appreciated too :-)

Comment 6

[Brian Crowder]

Checking in jsstr.c;
/cvsroot/mozilla/js/src/jsstr.c,v  <--  jsstr.c
new revision: 3.138; previous revision: 3.137
done