C++: Decompression Bombs #13560
C++: Decompression Bombs #13560
Conversation
|
QHelp previews: cpp/ql/src/experimental/Security/CWE/CWE-409-DecompressionBomb/DecompressionBomb.qhelperrors/warnings: |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * The `gzopen` function as a Flow source | ||
| */ | ||
| private class GzopenFunction extends Function { | ||
| GzopenFunction() { hasGlobalName("gzopen") } |
Check warning
Code scanning / CodeQL
Using implicit `this` Warning
| * The `gzdopen` function as a Flow source | ||
| */ | ||
| private class GzdopenFunction extends Function { | ||
| GzdopenFunction() { hasGlobalName("gzdopen") } |
Check warning
Code scanning / CodeQL
Using implicit `this` Warning
| * The `gzfread` function is used in Flow sink | ||
| */ | ||
| private class GzfreadFunction extends Function { | ||
| GzfreadFunction() { hasGlobalName("gzfread") } |
Check warning
Code scanning / CodeQL
Using implicit `this` Warning
| * The `gzread` function is used in Flow sink | ||
| */ | ||
| private class GzreadFunction extends Function { | ||
| GzreadFunction() { hasGlobalName("gzread") } |
Check warning
Code scanning / CodeQL
Using implicit `this` Warning
| * The `inflate`/`inflateSync` function is used in Flow sink | ||
| */ | ||
| private class DeflateFunction extends Function { | ||
| DeflateFunction() { hasGlobalName(["inflate", "inflateSync"]) } |
Check warning
Code scanning / CodeQL
Using implicit `this` Warning
| * The `uncompress`/`uncompress2` function is used in Flow sink | ||
| */ | ||
| private class UncompressFunction extends Function { | ||
| UncompressFunction() { hasGlobalName(["uncompress", "uncompress2"]) } |
Check warning
Code scanning / CodeQL
Using implicit `this` Warning
|
Hi, I've completed the work on this query and I don't have any further updates/commits here. |
|
Hi, currently for most of the libraries there are no sanitizers because I didn't know how can I set a limit on output buffer length for many of them, I think there is no way to control the output resource for some methods of these libs, the other problem is that being able to write proper sanitizer too which I succeeded to write one for |
|
@am0o0 how is this going? would you like me to open this PR to get the CodeQL team to look at it? |
|
@Kwstubbs yes, I can also open this PR if you can't. |
|
@am0o0 sounds good please do |
…into amammad-cpp-bombs
|
pinging @codeql-c-analysis query is ready for review. |
There was a problem hiding this comment.
Some initial comments. I will need to make a second pass over this.
The query should be relocated to cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409
The PR currently seems to be missing tests, which should be added to cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409. See the directories around there for examples of how to write tests.
cpp/ql/src/experimental/Security/CWE/CWE-409-DecompressionBomb/DecompressionBombs.ql
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-409-DecompressionBomb/Bzip2.qll
Outdated
Show resolved
Hide resolved
cpp/ql/src/experimental/Security/CWE/CWE-409-DecompressionBomb/DecompressionBombs.ql
Outdated
Show resolved
Hide resolved
| from DecompressionTaint::PathNode source, DecompressionTaint::PathNode sink | ||
| where DecompressionTaint::flowPath(source, sink) | ||
| select sink.getNode(), source, sink, "This Decompression output $@.", source.getNode(), | ||
| "is not limited" |
There was a problem hiding this comment.
Why will the examples from you example_good.cpp file not be flagged up by this query?
| totalRead += BUFFER_SIZE; | ||
| if (unzippedBytes > 0) { | ||
| unzippedData.insert(unzippedData.end(), unzipBuffer, unzipBuffer + unzippedBytes); | ||
| if (totalRead > 1024 * 1024 * 4) { |
There was a problem hiding this comment.
It would be interesting to add these examples as a test (in cpp/ql/test/experimental/query-tests/Security/CWE/...). I'm not clear how we intend to detect that a read limit such as this one is in place.
|
@jketema this directory doesn't exist:
|
Feel free to create it as part of this PR. |
|
Hi, as I don't have enough experience with C++ package/module systems, It takes some time to implement tests. |
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.
this query will be upgraded more in this week whether in this pull request or in another pull request, currently I'm adding minizip, xz, bzip2, zstd, LZ4 related libraries.
I'm trying my best to add proper sanitizers too if there is any protection for each function as some functions are unsafe by default and can not be controlled during decompression.