Closed Bug 467749 Opened 11 years ago Closed 10 years ago

possible valid coverity security bug CID: 1311 in nsZipArchive::BuildFileList


(Core :: Networking: JAR, defect)

Not set





(Reporter: guninski, Unassigned)


(Blocks 1 open bug)


(Keywords: coverity, Whiteboard: [sg:investigate])

coverity suspects CID: 1311 from run 279 is uninitialized read + possible buffer overflow.

the testcase will not be trivial and will depend on stack content, but inspection shows coverity may be right.

to cut a long story short:

[1] if (leftover < (namelen + extralen + commentlen + ZIPCENTRAL_SIZE)) {

[1] takes false path and |pos| seems to be nonzero.

982 pos += namelen + extralen + commentlen;
983 sig = xtolong(buf+pos);

983 seems outside of |buf| and we are back in the reading loop. for lucky value of |sig| further reading of |pos| is possible and in addition 
948 memcpy(buf, buf+pos, leftover);
probably may overflow.

may have missed something and this may be wrong.
Whiteboard: [sg:investigate]
Product: Firefox → Core
Summary: possible valid coverity security bug CID: 1311 in nsZipArchive.cpp → possible valid coverity security bug CID: 1311 in nsZipArchive::BuildFileList
QA Contact: general → general
Component: General → Networking: JAR
QA Contact: general → networking.jar
First of all the issue points to old code.
We now scan the buildfilelist from a memmapped file.
But in theory, one could have out-of-bounds reads here in the same way.

Thtere is at least some checking at:
590     if (endp - buf < ZIPCENTRAL_SIZE)
591       return ZIP_ERR_CORRUPT;

There is work underway and a ready patch for more JAR optimizations, such as bug 510844.

We could have some extra checks in the meanwhile to close this potential oob read.
Fixed by bug 510844.
Reads outside the memmapped space of the jar is no longer possible.
Closed: 10 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.