JS: Decompression Bombs

[am0o0]

This is part of All for one, one for all query submission, I'm going to submit an issue in github/securitylab for this pull request too.
I've added sanitizers as much as I could find safe ways of using methods of each library in their documentation.
I also added some additional taint steps which I think it is good to consider them as global steps too.
I tried my best to cover Command line flow sources as much as possible which can be used in other existing query too.
One interesting thing is that during finding CVEs for this submission I found out that there are some second level Remote flow sources like a situation that users upload their files and then get the file ID in one endpoint and their web app client will send another request containing this file ID to a second endpoint. the second endpoint mostly need a additional taint steps to work, one of these steps is Sequelize library Model. these additional steps are like second level Remote flow sources which is come after first level ones.

[smowton]

Anyone not watching the repo in general, note this is part of a family of submissions:

#13553
#13554
#13555
#13556
#13557
#13558

[am0o0]

Anyone not watching the repo in general, note this is part of a family of submissions:

#13553 #13554 #13555 #13556 #13557 #13558

#13560 is added too

[github-actions]

QHelp previews:

javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.qhelp

errors/warnings:

A fatal error occurred: Failed to read path ./javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql
(eventual cause: NoSuchFileException "./javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql")

[github-actions]

QHelp previews:

javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.qhelp

errors/warnings:

A fatal error occurred: Failed to read path ./javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql
(eventual cause: NoSuchFileException "./javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql")

[github-actions]

QHelp previews:

javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.qhelp

User-controlled file decompression

Extracting Compressed files with any compression algorithm like gzip can cause to denial of service attacks.

Attackers can compress a huge file which created by repeated similiar byte and convert it to a small compressed file.

Recommendation

When you want to decompress a user-provided compressed file you must be careful about the decompression ratio or read these files within a loop byte by byte to be able to manage the decompressed size in each cycle of the loop.

Example

JsZip: check uncompressedSize Object Field before extraction.

const jszipp = require("jszip");
function zipBombSafe(zipFile) {
    jszipp.loadAsync(zipFile.data).then(function (zip) {
        if (zip.file("10GB")["_data"]["uncompressedSize"] > 1024 * 1024 * 8) {
            console.log("error")
        }
        zip.file("10GB").async("uint8array").then(function (u8) {
            console.log(u8);
        });
    });
}

nodejs Zlib: use maxOutputLength option which it'll limit the buffer read size

const zlib = require("zlib");

zlib.gunzip(
    inputZipFile.data,
    { maxOutputLength: 1024 * 1024 * 5 },
    (err, buffer) => {
        doSomeThingWithData(buffer);
    });
zlib.gunzipSync(inputZipFile.data, { maxOutputLength: 1024 * 1024 * 5 });

inputZipFile.pipe(zlib.createGunzip({ maxOutputLength: 1024 * 1024 * 5 })).pipe(outputFile);

node-tar: use maxReadSize option which it'll limit the buffer read size

const tar = require("tar");

tar.x({
    file: tarFileName,
    strip: 1,
    C: 'some-dir',
    maxReadSize: 16 * 1024 * 1024 // 16 MB
})

References

• CVE-2017-16129
• A great research to gain more impact by this kind of attacks
• Common Weakness Enumeration: CWE-522.

[asgerf]

Hi @amammad 👋

Thanks for the many submissions. I haven't had a chance to review the impact, but it certainly seems very valuable to have these queries.

Before we get into language-specific reviews, there are a few recurring technical issues with the PRs:

• There seem to be some accidental file deletions/modifications of random unrelated files in the repo. Please undo those changes.
• Ensure the .qhelp file has the same name as the corresponding .ql file. Otherwise it just doesn't work and the PR can't be merged.
• Stick to one query per language.
  • The JS PR contains one query per library. They will need to be merged into one query.
  • The Ruby PR has two variants of the query. Please pick one of them to keep.

If you could address the above for all the PRs that would be help the review process a lot.

Lastly, once the above has been addressed, it would be great if you could mention when you think the PR is ready for review, at which point you should stop pushing new changes, unless in response to a review comment. It makes reviews very hard when new changes keep coming in.

[am0o0]

Hi @asgerf
My submissions are nothings compared to your works for developing codeql itself.
the reason of having multiple queries is that the sources and sinks are related to each one and if I merge all of these queries in one query it'll be a lot of additional nodes and maybe FP in my query. I've solved this problem in other languages with FlowState ( which still I don't know how much it is different from having multiple query file), IDK here I can do this or not, I'll look at this.
another problem is that some .qll files which I don't know where should I put them or I have a query file besides them to demonstrate them.

[am0o0]

Hi, I've completed the work on this query and I don't have any further updates/commits here.
one thing about this query is that there are two sources that only related to some sinks and they don't need to be exists for other sinks and I couldn't find a neat way to do merge all queries in one query perfectly .

[am0o0]

Hi @asgerf
Is it possible for you to review this PR before 25/3?
I have two weeks of forced vacation from 25/03 so I can focus on finishing this and other codeql PRs.

[asgerf]

Since this targets files exclusively within experimental, I'm going to merge without substantial review.

Thanks for your contribution, and apologies for the long wait.

[am0o0]

Sorry for the delay, approved.

Thank you a lot for approving this PR and developing and improving the CodeQL every day :)