contrib (minizip): potential buffer overflow in zipRemoveExtraInfoBlock

[mswilson]

Back in 2019 @FelixVonLeitner (of dietlibc fame) filed a bug in the Info-Zip SourceForge tracker: https://sourceforge.net/p/infozip/bugs/57/ saying:

1962 pNewHeader = (char)ALLOC(dataLen);
1963 pTmp = pNewHeader;
1964
1965 while(p < (pData + dataLen))
1966 {
1967 header = (short)p;
1968 dataSize = (((short*)p)+1);

At this point the code assumes that dataSize < *dataLen. An attacker could be handing you an malicious ZIP file with corrupt data however.

1977 memcpy(pTmp, p, dataSize + 4);

In which case this is a heap buffer overflow.

I think the report never made its way to @gvollant and @madler.

Increased scanning automation is likely to flag this repeatedly, e.g., https://github.com/Swival/security-audits/blob/b68db289f3d97aee434d7825701dac6089449345/zlib/009-extra-field-parser-trusts-attacker-controlled-lengths-and-ov.md (note, this report seems to have some inaccuracies about reachability in the context of the minizip code itself.)

This seems to be a rarely used function, at least in open-source software. One example of use I've found is the Lua wrapper for minizip: https://github.com/luapower/minizip/blob/c3bc1ff8d1adece5861be32bbb3e020b13ffdca2/minizip.lua#L311

Perhaps @mathiassv knows of other uses?

[mswilson]

Though the (presumed AI-assisted) scanning report above isn't perfect, a cursory reading of the accompanying patch looks ok. https://github.com/Swival/security-audits/blob/b68db289f3d97aee434d7825701dac6089449345/zlib/009-extra-field-parser-trusts-attacker-controlled-lengths-and-ov.patch

[mswilson]

See also PR #1259

[mswilson]

See also PR #1248

[Neustradamus]

@gvollant: ^