georgi - hopefully not receiving bugspam Reporter
	Description • 16 years ago

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:

http://hg.mozilla.org/mozilla-central/annotate/32c453ca17ed/modules/libjar/nsZipArchive.cpp#l945

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

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

later:
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.

	Alfred Kayser
	Comment 1 • 15 years ago

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.

	Alfred Kayser
	Comment 2 • 15 years ago

Fixed by bug 510844.
Reads outside the memmapped space of the jar is no longer possible.