From f97b1039cd3a1f19d18ef1a775dbd66eef08bb19 Mon Sep 17 00:00:00 2001 From: am0o0 <77095239+am0o0@users.noreply.github.com> Date: Tue, 30 Jul 2024 17:49:34 +0200 Subject: [PATCH] update test files, add one more additional flow step for inflate function, fix gzopen additional flow step thanks to @jketema --- .../CWE/CWE-409/DecompressionBombs.ql | 5 +-- .../Security/CWE/CWE-409/ZlibGzopen.qll | 4 +-- .../Security/CWE/CWE-409/ZlibInflator.qll | 19 +++++++++-- .../Security/CWE/CWE-409/zlibTest.cpp | 32 +++---------------- 4 files changed, 26 insertions(+), 34 deletions(-) diff --git a/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/DecompressionBombs.ql b/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/DecompressionBombs.ql index 896c15f7a80..05f39c47e13 100644 --- a/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/DecompressionBombs.ql +++ b/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/DecompressionBombs.ql @@ -22,12 +22,13 @@ module DecompressionTaintConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { exists(FunctionCall fc, DecompressionFunction f | fc.getTarget() = f | - fc.getArgument(f.getArchiveParameterIndex()) = sink.asExpr() + fc.getArgument(f.getArchiveParameterIndex()) = sink.asExpr() ) } predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { - any(DecompressionFlowStep f).isAdditionalFlowStep(node1, node2) + any(DecompressionFlowStep f).isAdditionalFlowStep(node1, node2) or + nextInAdditionalFlowStep(node1, node2) } } diff --git a/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/ZlibGzopen.qll b/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/ZlibGzopen.qll index 5c4f367b1b4..edb8ef7ff68 100644 --- a/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/ZlibGzopen.qll +++ b/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/ZlibGzopen.qll @@ -37,7 +37,7 @@ class GzGetsFunction extends DecompressionFunction { class GzReadFunction extends DecompressionFunction { GzReadFunction() { this.hasGlobalName("gzread") } - override int getArchiveParameterIndex() { result = 1 } + override int getArchiveParameterIndex() { result = 0 } } /** @@ -66,7 +66,7 @@ class GzopenFunction extends DecompressionFlowStep { override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(FunctionCall fc | fc.getTarget() = this | - node1.asExpr() = fc.getArgument(0) and + node1.asIndirectExpr() = fc.getArgument(0) and node2.asExpr() = fc ) } diff --git a/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/ZlibInflator.qll b/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/ZlibInflator.qll index 1897b8e09d3..464dce3ac45 100644 --- a/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/ZlibInflator.qll +++ b/cpp/ql/src/experimental/query-tests/Security/CWE/CWE-409/ZlibInflator.qll @@ -10,12 +10,27 @@ import DecompressionBomb /** * The `inflate` and `inflateSync` functions are used in flow sink. * - * `inflate(z_streamp strm, int flush)` + * `inflate(z_stream strm, int flush)` * - * `inflateSync(z_streamp strm)` + * `inflateSync(z_stream strm)` */ class InflateFunction extends DecompressionFunction { InflateFunction() { this.hasGlobalName(["inflate", "inflateSync"]) } override int getArchiveParameterIndex() { result = 0 } } + +/** + * The `next_in` member of a `z_stream` variable is used in flow steps. + */ +predicate nextInAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(Variable nextInVar, VariableAccess zStreamAccess | + nextInVar.getDeclaringType().hasName("z_stream") and + nextInVar.hasName("next_in") and + zStreamAccess.getType().hasName("z_stream") + | + nextInVar.getAnAccess().getQualifier().(VariableAccess).getTarget() = zStreamAccess.getTarget() and + node1.asIndirectExpr() = nextInVar.getAnAssignedValue() and + node2.asExpr() = zStreamAccess + ) +} diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409/zlibTest.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409/zlibTest.cpp index b829cfaa61d..07805f6c83f 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409/zlibTest.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409/zlibTest.cpp @@ -51,40 +51,17 @@ namespace std { } int UnsafeInflate(char *a) { - // placeholder for the compressed (deflated) version of "a" - char b[50]; - // placeholder for the Uncompressed (inflated) version of "b" - char c[50]; + // placeholder for the Uncompressed (inflated) version of "a" + char c[1024000]; - // STEP 1. - // zlib struct - z_stream defstream; - defstream.zalloc = Z_NULL; - defstream.zfree = Z_NULL; - defstream.opaque = Z_NULL; - // setup "a" as the input and "b" as the compressed output - defstream.avail_in = (uInt) 50 + 1; // size of input, string + terminator - defstream.next_in = (Bytef *) a; // input char array - defstream.avail_out = (uInt) sizeof(b); // size of output - defstream.next_out = (Bytef *) b; // output char array - - // the actual compression work. - deflateInit(&defstream, Z_BEST_COMPRESSION); - deflate(&defstream, Z_FINISH); - deflateEnd(&defstream); - - // This is one way of getting the size of the output - // STEP 2. - // inflate b into c - // zlib struct z_stream infstream; infstream.zalloc = Z_NULL; infstream.zfree = Z_NULL; infstream.opaque = Z_NULL; // setup "b" as the input and "c" as the compressed output // TOTHINK: Here we can add additional step from Right operand to z_stream variable access - infstream.avail_in = (uInt) ((char *) defstream.next_out - b); // size of input - infstream.next_in = (Bytef *) b; // input char array + infstream.avail_in = (uInt) (1000); // size of input + infstream.next_in = (Bytef *) a; // input char array infstream.avail_out = (uInt) sizeof(c); // size of output infstream.next_out = (Bytef *) c; // output char array @@ -159,7 +136,6 @@ int UnsafeGzgets(char *fileName) { } char *buffer = new char[4000000000]; char *result; - result = gzgets(inFileZ, buffer, 1000000000); while (true) { result = gzgets(inFileZ, buffer, 1000000000); if (result == nullptr) {