[BUG] node-tar crashes when extracting tar created with tar-stream containing long linkpath

[forty]

What / Why

node-tar crashes when extracting tar files created with tar-stream which contains long linkpath

I'm not sure if the tar files created by tar-stream are somehow invalid (the tar format seems to be a mess, and I don't know enough to figure it out), but I'm reporting here for 2 reasons:

• crashing does not seem to be a great way to react to this. seems like it could cause security issue (DOS) if node-tar is used to extract untrusted tar files
• the tar files with long linkpath produced by tar-stream are extracted correctly by GNU tar on my system

When

• create a tar file using tar-stream containing a symlink pointing to a long linkpath
• extract this file using node-tar
• see that it crashes

Where

Such tar makes npm install file.tar crash, but it's just an example

How

Steps to Reproduce

Install dependencies

$ # node >= 12
$ npm install [email protected]
$ npm install [email protected]

Run this script

const tar = require('tar');
const tarStream = require('tar-stream');
const fs = require('fs');
const { spawn } = require('child_process');

const makeTestFile = () => {
    var pack = tarStream.pack();
    pack.entry({ name: 'test', type: 'symlink', linkname: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaverylongname' });
    pack.finalize()
    return pack;
};

const makeTestDir = () => {
    return fs.promises.mkdtemp('test-tar-');
}

const testTarListWithTarModule = async () => {
    return new Promise((resolve, reject) => {
        process.on('uncaughtException', reject);

        const file = makeTestFile();
        const pipe = file.pipe(tar.list());

        const results = [];
        pipe.on('entry', entry => {
            results.push(entry);
        });
        pipe.on('error', reject);
        pipe.on('end', () => resolve(results.map(r => {
            return `${r.type} ${r.mode} ${r.uid}/${r.gid} ${r.size} ${r.mtime.toISOString()} ${r.path} -> ${r.linkpath}`
        }).join('\n' )));
    });
};

const testTarExtractWithTarModule = async () => {
    return new Promise(async (resolve, reject) => {
        process.on('uncaughtException', reject);

        const file = makeTestFile();
        const dir = await makeTestDir();
        const pipe = file.pipe(tar.extract({cwd: dir}));
        pipe.on('error', reject);
        pipe.on('end', resolve);
    });
};

const testTarListWithTarBinary = async () => {
    return new Promise((resolve, reject) => {
        process.on('uncaughtException', reject);

        const file = makeTestFile();

        const tarCmd = spawn('tar', ['-tv' ]);
        const pipe = file.pipe(tarCmd.stdin);

        const results = [];
        tarCmd.stdout.on('data', data => {
            results.push(data);
        });
        tarCmd.stderr.on('data', data => {
            results.push(data);
        });
        pipe.on('error', reject);
        tarCmd.on('close', (code) => {
            if(code !== 0) {
                reject(new Error(`Failed with status code ${code}`))
            } else {
                resolve(Buffer.concat(results).toString('utf-8'));
            }
        });
    });
};

const testTarExtractWithTarBinary = async () => {
    return new Promise(async (resolve, reject) => {
        process.on('uncaughtException', reject);

        const file = makeTestFile();
        const dir = await makeTestDir();

        const tarCmd = spawn('tar', ['-xv', '-C', dir ]);
        const pipe = file.pipe(tarCmd.stdin);

        const results = [];
        tarCmd.stdout.on('data', data => {
            results.push(data);
        });
        tarCmd.stderr.on('data', data => {
            results.push(data);
        });
        pipe.on('error', reject);
        tarCmd.on('close', (code) => {
            if(code !== 0) {
                reject(new Error(`Failed with status code ${code}`))
            } else {
                resolve(Buffer.concat(results).toString('utf-8'))
            }
        });
    });
};


const doTest = (name, test) => {
    return test().then((result) => {
        console.log(`${name} success, with result: `);
        console.log('----------------------------------------------------------');
        console.log(result);
        console.log('----------------------------------------------------------');
        console.log('')
    }).catch((error) => {
        console.log(`${name} failure, with error: `);
        console.log('----------------------------------------------------------');
        console.log(error);
        console.log('----------------------------------------------------------');
        console.log('')
    });
};

process.stdin.resume();
(async () => {
    await doTest('testTarListtWithTarModule', testTarListWithTarModule);
    await doTest('testTarListWithTarBinary', testTarListWithTarBinary);
    await doTest('testTarExtractWithTarModule', testTarExtractWithTarModule);
    await doTest('testTarExtractWithTarBinary', testTarExtractWithTarBinary);

    process.exit(0);
})();

Current Behavior

Output of the script

testTarListtWithTarModule success, with result: 
----------------------------------------------------------
SymbolicLink 420 0/0 0 2022-03-17T10:49:12.000Z PaxHeader -> PaxHeader
----------------------------------------------------------

testTarListWithTarBinary success, with result: 
----------------------------------------------------------
lrw-r--r-- 0/0               0 2022-03-17 11:49 test -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaverylongname

----------------------------------------------------------

testTarExtractWithTarModule failure, with error: 
----------------------------------------------------------
TypeError: Cannot read property '0' of undefined
    at /home/qbarbe/Devel/tar-tests/node_modules/tar/lib/path-reservations.js:58:30
    at Array.every (<anonymous>)
    at check (/home/qbarbe/Devel/tar-tests/node_modules/tar/lib/path-reservations.js:58:18)
    at run (/home/qbarbe/Devel/tar-tests/node_modules/tar/lib/path-reservations.js:64:29)
    at /home/qbarbe/Devel/tar-tests/node_modules/tar/lib/path-reservations.js:107:24
    at Set.forEach (<anonymous>)
    at clear (/home/qbarbe/Devel/tar-tests/node_modules/tar/lib/path-reservations.js:107:10)
    at /home/qbarbe/Devel/tar-tests/node_modules/tar/lib/path-reservations.js:67:14
    at done (/home/qbarbe/Devel/tar-tests/node_modules/tar/lib/unpack.js:570:7)
    at /home/qbarbe/Devel/tar-tests/node_modules/tar/lib/unpack.js:685:7
----------------------------------------------------------

testTarExtractWithTarBinary success, with result: 
----------------------------------------------------------
test

----------------------------------------------------------

Expected Behavior

testTarExtractWithTarModule test does not fail

Who

N/A

References

N/A

[isaacs]

Thanks! Turns out this was actually a pretty interesting and subtle error. Tar-stream is making a curious choice, but it's not technically invalid, and should be supported.