|
| 1 | +'use strict'; |
| 2 | + |
| 3 | +// Security-hardening regression tests for node:zlib ZIP support. Each test |
| 4 | +// describes a distinct issue found by audit and asserts the *secure* behavior, |
| 5 | +// so every test fails on the pre-fix code and passes once its fix lands. |
| 6 | +// |
| 7 | +// 1. Local-vs-central header confusion (parser-confusion / inspect-then-consume |
| 8 | +// divergence): the central directory is authoritative in this reader, but a |
| 9 | +// local file header that disagrees on method, sizes, CRC, or the encryption |
| 10 | +// flag lets another ZIP tool (which extracts from the local header) read a |
| 11 | +// different member from the same archive. Such an archive must be rejected. |
| 12 | +// 2. The archiver (zipFiles) must not block forever on a FIFO/special source. |
| 13 | +// 3. The streaming read path (contentIterator) is hard-bounded by the header's |
| 14 | +// declared uncompressed size: a member that inflates past it is rejected |
| 15 | +// mid-stream, so entry.size is a ceiling a consumer can trust up front. |
| 16 | + |
| 17 | +require('../common'); |
| 18 | +constassert=require('assert'); |
| 19 | +const{ test }=require('node:test'); |
| 20 | +constzlib=require('zlib'); |
| 21 | +constpath=require('path'); |
| 22 | +const{ spawnSync }=require('child_process'); |
| 23 | +consttmpdir=require('../common/tmpdir'); |
| 24 | + |
| 25 | +constSIG_LOCAL=0x04034b50; |
| 26 | +constSIG_CENTRAL=0x02014b50; |
| 27 | + |
| 28 | +// A minimal single-member archive; caller patches its headers. |
| 29 | +functionbuildStored(name,content,method='store'){ |
| 30 | +constentry=zlib.ZipEntry.createSync(name,Buffer.from(content),{ method }); |
| 31 | +constchunks=[]; |
| 32 | +for(constchunkofzlib.createZipArchiveSync([entry]))chunks.push(chunk); |
| 33 | +returnBuffer.concat(chunks); |
| 34 | +} |
| 35 | + |
| 36 | +functioncentralOffset(buf){ |
| 37 | +for(leti=buf.length-22;i>=0;i--){ |
| 38 | +if(buf.readUInt32LE(i)===SIG_CENTRAL)returni; |
| 39 | +} |
| 40 | +thrownewError('no central directory header found'); |
| 41 | +} |
| 42 | + |
| 43 | +// A ZipBuffer parses the central directory; reading a member resolves and |
| 44 | +// checks its local header. Do both so the assertion holds whether the check |
| 45 | +// is eager (parse time) or lazy (read time). |
| 46 | +functionreadsThrow(buf,name){ |
| 47 | +assert.throws(()=>{ |
| 48 | +constzb=newzlib.ZipBuffer(buf); |
| 49 | +zb.get(name).contentSync(); |
| 50 | +},{code: 'ERR_ZIP_INVALID_ARCHIVE'}); |
| 51 | +} |
| 52 | + |
| 53 | +// 1a. Local vs central compressed/uncompressed size disagreement. |
| 54 | +test('a local/central size disagreement is rejected',()=>{ |
| 55 | +constbuf=buildStored('a.txt','hello'); |
| 56 | +assert.strictEqual(buf.readUInt32LE(0),SIG_LOCAL); |
| 57 | +buf.writeUInt32LE(999,18);// Local compressed size |
| 58 | +buf.writeUInt32LE(999,22);// Local uncompressed size |
| 59 | +readsThrow(buf,'a.txt'); |
| 60 | +}); |
| 61 | + |
| 62 | +// 1b. Local vs central compression-method disagreement. |
| 63 | +test('a local/central method disagreement is rejected',()=>{ |
| 64 | +constbuf=buildStored('a.txt','hello'); |
| 65 | +buf.writeUInt16LE(8,8);// Local method deflate; central stays store(0) |
| 66 | +readsThrow(buf,'a.txt'); |
| 67 | +}); |
| 68 | + |
| 69 | +// 1c. Central marks the member encrypted while the local header does not: |
| 70 | +// a central-directory reader (e.g. python) treats it as opaque/encrypted, so |
| 71 | +// Node must not silently decode it either. |
| 72 | +test('a local/central encryption-flag disagreement is rejected',()=>{ |
| 73 | +constbuf=buildStored('a.txt','hello'); |
| 74 | +constc=centralOffset(buf); |
| 75 | +buf.writeUInt16LE(buf.readUInt16LE(c+8)|0x0001,c+8);// Central encrypted bit |
| 76 | +readsThrow(buf,'a.txt'); |
| 77 | +}); |
| 78 | + |
| 79 | +// 2. zipFiles must reject a FIFO source rather than block on open() forever. |
| 80 | +test('zipFiles rejects a FIFO source instead of hanging',()=>{ |
| 81 | +if(process.platform==='win32')return;// no mkfifo |
| 82 | +tmpdir.refresh(); |
| 83 | +constfifo=path.join(tmpdir.path,'evil.fifo'); |
| 84 | +if(spawnSync('mkfifo',[fifo]).status!==0)return;// mkfifo unavailable |
| 85 | +constscript= |
| 86 | +'const zlib = require("zlib");'+ |
| 87 | +'(async () => {'+ |
| 88 | +' try {'+ |
| 89 | +` for await (const _ of zlib.zipFiles([[${JSON.stringify(fifo)}, "x"]], `+ |
| 90 | +' { followSymlinks: false })) {}'+ |
| 91 | +' console.log("COMPLETED");'+ |
| 92 | +' } catch (e) { console.log("REJECTED:" + e.code); }'+ |
| 93 | +'})();'; |
| 94 | +constres=spawnSync(process.execPath,['--no-warnings','-e',script], |
| 95 | +{timeout: 5000,encoding: 'utf8'}); |
| 96 | +assert.ok(res.signal===null, |
| 97 | +'zipFiles hung on a FIFO source (killed by timeout)'); |
| 98 | +assert.match(res.stdout,/REJECTED:ERR_ZIP_UNSUPPORTED_FEATURE/); |
| 99 | +}); |
| 100 | + |
| 101 | +// 3. Streaming is hard-bounded by the declared uncompressed size: a member |
| 102 | +// whose data inflates past it is rejected mid-stream, so a consumer can trust |
| 103 | +// entry.size as the ceiling before choosing to buffer. |
| 104 | +test('contentIterator rejects a member that inflates past its declared size',async()=>{ |
| 105 | +constbig=zlib.ZipEntry.createSync( |
| 106 | +'big',Buffer.alloc(64*1024),{method: 'deflate'}); |
| 107 | +constchunks=[]; |
| 108 | +for(constchunkofzlib.createZipArchiveSync([big]))chunks.push(chunk); |
| 109 | +constbuf=Buffer.concat(chunks); |
| 110 | +// Shrink the declared uncompressed size in both headers (kept consistent so |
| 111 | +// the header cross-check passes) below what the data actually inflates to. |
| 112 | +constc=centralOffset(buf); |
| 113 | +buf.writeUInt32LE(100,22);// Local uncompressed size |
| 114 | +buf.writeUInt32LE(100,c+24);// Central uncompressed size |
| 115 | +constentry=newzlib.ZipBuffer(buf).get('big'); |
| 116 | +awaitassert.rejects(async()=>{ |
| 117 | +// eslint-disable-next-line no-unused-vars |
| 118 | +forawait(const_ofentry.contentIterator()){/* drain */} |
| 119 | +},{code: 'ERR_ZIP_ENTRY_CORRUPT',message: /inflatesbeyonditsdeclaredsize/}); |
| 120 | +}); |
0 commit comments