Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

msgpackr may silently replace 64 byte+ strings with empty string after encoding 2GiB messages #132

Open
akumansley opened this issue Feb 16, 2024 · 1 comment

Comments

@akumansley
Copy link

akumansley commented Feb 16, 2024

Hi!

We believe we've run into an issue where the msgpackr module can become "poisoned" after encoding a message longer than 2GiB.

Here's a test case that reproduces the issue on OS X Node.js v18.16.0. We've also encountered the issue on Linux Node.js v18.16.0.

const msgpackr = require('msgpackr');

const obj = {};

const largeString = 'a'.repeat(0x100);

const size = 2 ** 30; // 1GiB
const numberOfStringsRequiredFor1GB = Math.ceil(size / largeString.length);

for (let i = 0; i < numberOfStringsRequiredFor1GB; i++) {
    const key = i.toString();
    obj[key] = largeString;
}

// create a too-large target buffer
const packr = new msgpackr.Packr();

// incidentally, this value will be encoded correctly, because Buffer.utf8Write
// will only fail to write towards the beginning of the buffer
const _ = packr.pack([obj, obj]);

// now the packer module is poisoned
const newPackr = new msgpackr.Packr();

// must be at least a 64ch string to trigger the bug
const modestString = 'a'.repeat(64);
const simpleObj = {name: modestString};
const incorrectlyEncoded = newPackr.pack(simpleObj);
const decodedIncorrectlyEncoded = newPackr.unpack(incorrectlyEncoded);

if (decodedIncorrectlyEncoded.name !== modestString) {
    throw new Error("expected 64 'a's, but found ", decodedIncorrectlyEncoded.name);
}

As a workaround, we're calling packr.useBuffer(Buffer.allocUnsafeSlow(8192)) to force the packr to reset its internal target buffer.

@adembo
Copy link

adembo commented Feb 21, 2024

We filed a bug report with NodeJS for what we believe to be the root cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants