From bb7b8e1ad0e345862980a163b688f4554ebc1df8 Mon Sep 17 00:00:00 2001 From: Sakshis Date: Mon, 16 Dec 2024 13:09:06 +0000 Subject: [PATCH 1/7] removed missing-secure-java --- rules/java/security/missing-secure-java.yml | 70 ------------------- .../missing-secure-java-snapshot.yml | 32 --------- tests/java/missing-secure-java-test.yml | 15 ---- 3 files changed, 117 deletions(-) delete mode 100644 rules/java/security/missing-secure-java.yml delete mode 100644 tests/__snapshots__/missing-secure-java-snapshot.yml delete mode 100644 tests/java/missing-secure-java-test.yml diff --git a/rules/java/security/missing-secure-java.yml b/rules/java/security/missing-secure-java.yml deleted file mode 100644 index 755e6660..00000000 --- a/rules/java/security/missing-secure-java.yml +++ /dev/null @@ -1,70 +0,0 @@ -id: missing-secure-java -language: java -severity: warning -message: >- - Detected a cookie where the `Secure` flag is either missing or - disabled. The `Secure` cookie flag instructs the browser to forbid sending - the cookie over an insecure HTTP request. Set the `Secure` flag to `true` - so the cookie will only be sent over HTTPS. -note: >- - [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute - [OWASP A05:2021]: Security Misconfiguration - [REFERENCES] - - https://owasp.org/Top10/A05_2021-Security_Misconfiguration -utils: - match_without_httponly: - kind: argument_list - has: - kind: object_creation_expression - inside: - stopBy: end - kind: method_invocation - - match_cookie_last: - kind: argument_list - has: - kind: method_invocation - has: - kind: argument_list - has: - kind: string_literal - - match_instance: - kind: local_variable_declaration - has: - stopBy: end - kind: identifier - follows: - stopBy: end - kind: variable_declarator - - match_identifier_with_simplecookie: - kind: identifier - inside: - stopBy: end - kind: local_variable_declaration - all: - - has: - stopBy: end - kind: type_identifier - regex: '^SimpleCookie$|^Cookie$' - - has: - stopBy: neighbor - kind: variable_declarator - all: - - has: - stopBy: neighbor - kind: identifier - - has: - stopBy: neighbor - kind: object_creation_expression - - not: - precedes: - stopBy: neighbor - kind: expression_statement -rule: - any: - - matches: match_instance - - matches: match_without_httponly - - matches: match_cookie_last - - matches: match_identifier_with_simplecookie diff --git a/tests/__snapshots__/missing-secure-java-snapshot.yml b/tests/__snapshots__/missing-secure-java-snapshot.yml deleted file mode 100644 index 3931463b..00000000 --- a/tests/__snapshots__/missing-secure-java-snapshot.yml +++ /dev/null @@ -1,32 +0,0 @@ -id: missing-secure-java -snapshots: - ? | - SimpleCookie s = new SimpleCookie("foo", "bar"); - .orElse( new NettyCookie( "foo", "bar" ) ); - Cookie z = new NettyCookie("foo", "bar"); - return HttpResponse.ok().cookie(Cookie.of("zzz", "ddd")); - : labels: - - source: s - style: primary - start: 13 - end: 14 - - source: SimpleCookie - style: secondary - start: 0 - end: 12 - - source: s - style: secondary - start: 13 - end: 14 - - source: new SimpleCookie("foo", "bar") - style: secondary - start: 17 - end: 47 - - source: s = new SimpleCookie("foo", "bar") - style: secondary - start: 13 - end: 47 - - source: SimpleCookie s = new SimpleCookie("foo", "bar"); - style: secondary - start: 0 - end: 48 diff --git a/tests/java/missing-secure-java-test.yml b/tests/java/missing-secure-java-test.yml deleted file mode 100644 index 507f951f..00000000 --- a/tests/java/missing-secure-java-test.yml +++ /dev/null @@ -1,15 +0,0 @@ -id: missing-secure-java -valid: - - | - Cookie c1 = getCookieSomewhere(); - return HttpResponse.ok().cookie(Cookie.of("foo", "bar").secure(true)); - Cookie cookie = request.getCookies().findCookie( "foobar" ) - Cookie c = new NettyCookie("foo", "bar"); - c.secure(true); - NettyCookie r = new NettyCookie("foo", "bar").secure(true); -invalid: - - | - SimpleCookie s = new SimpleCookie("foo", "bar"); - .orElse( new NettyCookie( "foo", "bar" ) ); - Cookie z = new NettyCookie("foo", "bar"); - return HttpResponse.ok().cookie(Cookie.of("zzz", "ddd")); From 12bb3aab8d57915cd459d2e2ac04c42dfb2dca48 Mon Sep 17 00:00:00 2001 From: ESS ENN Date: Wed, 22 Jan 2025 18:58:13 +0530 Subject: [PATCH 2/7] httponly-false-csharp --- rules/csharp/security/httponly-false-csharp | 48 +++++++++++++++++++++ tests/csharp/httponly-false-csharp-test.yml | 9 ++++ 2 files changed, 57 insertions(+) create mode 100644 rules/csharp/security/httponly-false-csharp create mode 100644 tests/csharp/httponly-false-csharp-test.yml diff --git a/rules/csharp/security/httponly-false-csharp b/rules/csharp/security/httponly-false-csharp new file mode 100644 index 00000000..af939938 --- /dev/null +++ b/rules/csharp/security/httponly-false-csharp @@ -0,0 +1,48 @@ +id: httponly-false-csharp +language: csharp +severity: warning +message: >- + "Detected a cookie where the `HttpOnly` flag is either missing or + disabled. The `HttpOnly` cookie flag instructs the browser to forbid + client-side JavaScript to read the cookie. If JavaScript interaction is + required, you can ignore this finding. However, set the `HttpOnly` flag to + `true` in all other cases. If this wasn't intentional, it's recommended to + set the HttpOnly flag to true so the cookie will not be accessible through + client-side scripts or to use the Cookie Policy Middleware to globally set + the HttpOnly flag. You can then use the CookieOptions class when + instantiating the cookie, which inherits these settings and will require + future developers to have to explicitly override them on a case-by-case + basis if needed. This approach ensures cookies are secure by default." +note: >- + [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag" + [REFERENCES] + - https://learn.microsoft.com/en-us/aspnet/core/security/authentication/cookie?view=aspnetcore-8.0#cookie-policy-middleware + - https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.cookieoptions + - https://owasp.org/Top10/A05_2021-Security_Misconfiguration + + +ast-grep-essentials: true + +rule: + kind: boolean_literal + pattern: $LITERAL + follows: + regex: ^=$ + follows: + kind: member_access_expression + inside: + kind: assignment_expression + all: + - has: + kind: member_access_expression + nthChild: 1 + regex: \.Cookie$ + - has: + kind: identifier + nthChild: 2 + regex: ^HttpOnly$ + +constraints: + LITERAL: + regex: ^false$ + diff --git a/tests/csharp/httponly-false-csharp-test.yml b/tests/csharp/httponly-false-csharp-test.yml new file mode 100644 index 00000000..e29a7eab --- /dev/null +++ b/tests/csharp/httponly-false-csharp-test.yml @@ -0,0 +1,9 @@ +id: httponly-false-csharp +valid: + - | + myHttpOnlyCookie.HttpOnly = true; + - | + options.Cookie.HttpOnly = true; +invalid: + - | + options.Cookie.HttpOnly = false; From 2c5ea88476cdca70b993026ce65cb1435e602119 Mon Sep 17 00:00:00 2001 From: ESS ENN Date: Wed, 22 Jan 2025 19:00:33 +0530 Subject: [PATCH 3/7] use-of-md5-digest-utils-java --- .../security/use-of-md5-digest-utils-java.yml | 42 +++++++++++++++++++ .../use-of-md5-digest-utils-java-snapshot.yml | 29 +++++++++++++ .../use-of-md5-digest-utils-java-test.yml | 7 ++++ 3 files changed, 78 insertions(+) create mode 100644 rules/java/security/use-of-md5-digest-utils-java.yml create mode 100644 tests/__snapshots__/use-of-md5-digest-utils-java-snapshot.yml create mode 100644 tests/java/use-of-md5-digest-utils-java-test.yml diff --git a/rules/java/security/use-of-md5-digest-utils-java.yml b/rules/java/security/use-of-md5-digest-utils-java.yml new file mode 100644 index 00000000..553bac8a --- /dev/null +++ b/rules/java/security/use-of-md5-digest-utils-java.yml @@ -0,0 +1,42 @@ +id: use-of-md5-digest-utils-java +language: java +severity: warning +message: >- + 'Detected MD5 hash algorithm which is considered insecure. MD5 is not + collision resistant and is therefore not suitable as a cryptographic + signature. Use HMAC instead.' +note: >- + [CWE-328] Use of Weak Hash + [REFERENCES] + - https://owasp.org/Top10/A02_2021-Cryptographic_Failures + +ast-grep-essentials: true + +rule: + kind: identifier + regex: ^getMd5Digest$ + nthChild: 2 + precedes: + nthChild: 3 + kind: argument_list + not: + has: + nthChild: 1 + inside: + kind: method_invocation + nthChild: 1 + inside: + kind: method_invocation + all: + - has: + kind: identifier + nthChild: 2 + regex: ^digest$ + - has: + kind: argument_list + nthChild: 3 + - not: + has: + stopBy: end + kind: ERROR + diff --git a/tests/__snapshots__/use-of-md5-digest-utils-java-snapshot.yml b/tests/__snapshots__/use-of-md5-digest-utils-java-snapshot.yml new file mode 100644 index 00000000..2e74b70e --- /dev/null +++ b/tests/__snapshots__/use-of-md5-digest-utils-java-snapshot.yml @@ -0,0 +1,29 @@ +id: use-of-md5-digest-utils-java +snapshots: + ? | + byte[] hashValue = DigestUtils.getMd5Digest().digest(password.getBytes()); + : labels: + - source: getMd5Digest + style: primary + start: 31 + end: 43 + - source: digest + style: secondary + start: 46 + end: 52 + - source: (password.getBytes()) + style: secondary + start: 52 + end: 73 + - source: DigestUtils.getMd5Digest().digest(password.getBytes()) + style: secondary + start: 19 + end: 73 + - source: DigestUtils.getMd5Digest() + style: secondary + start: 19 + end: 45 + - source: () + style: secondary + start: 43 + end: 45 diff --git a/tests/java/use-of-md5-digest-utils-java-test.yml b/tests/java/use-of-md5-digest-utils-java-test.yml new file mode 100644 index 00000000..769a4b52 --- /dev/null +++ b/tests/java/use-of-md5-digest-utils-java-test.yml @@ -0,0 +1,7 @@ +id: use-of-md5-digest-utils-java +valid: + - | + byte[] hashValue = DigestUtils.getSha512Digest().digest(password.getBytes()); +invalid: + - | + byte[] hashValue = DigestUtils.getMd5Digest().digest(password.getBytes()); From d3067f11ba31741fd738392f2d2efb1702116dcf Mon Sep 17 00:00:00 2001 From: ESS ENN Date: Wed, 22 Jan 2025 19:05:33 +0530 Subject: [PATCH 4/7] removing use-of-md5-digest-utils and httponly-false-csharp --- rules/csharp/security/httponly-false-csharp | 48 ------------------- .../security/use-of-md5-digest-utils-java.yml | 42 ---------------- tests/csharp/httponly-false-csharp-test.yml | 9 ---- .../use-of-md5-digest-utils-java-test.yml | 7 --- 4 files changed, 106 deletions(-) delete mode 100644 rules/csharp/security/httponly-false-csharp delete mode 100644 rules/java/security/use-of-md5-digest-utils-java.yml delete mode 100644 tests/csharp/httponly-false-csharp-test.yml delete mode 100644 tests/java/use-of-md5-digest-utils-java-test.yml diff --git a/rules/csharp/security/httponly-false-csharp b/rules/csharp/security/httponly-false-csharp deleted file mode 100644 index af939938..00000000 --- a/rules/csharp/security/httponly-false-csharp +++ /dev/null @@ -1,48 +0,0 @@ -id: httponly-false-csharp -language: csharp -severity: warning -message: >- - "Detected a cookie where the `HttpOnly` flag is either missing or - disabled. The `HttpOnly` cookie flag instructs the browser to forbid - client-side JavaScript to read the cookie. If JavaScript interaction is - required, you can ignore this finding. However, set the `HttpOnly` flag to - `true` in all other cases. If this wasn't intentional, it's recommended to - set the HttpOnly flag to true so the cookie will not be accessible through - client-side scripts or to use the Cookie Policy Middleware to globally set - the HttpOnly flag. You can then use the CookieOptions class when - instantiating the cookie, which inherits these settings and will require - future developers to have to explicitly override them on a case-by-case - basis if needed. This approach ensures cookies are secure by default." -note: >- - [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag" - [REFERENCES] - - https://learn.microsoft.com/en-us/aspnet/core/security/authentication/cookie?view=aspnetcore-8.0#cookie-policy-middleware - - https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.cookieoptions - - https://owasp.org/Top10/A05_2021-Security_Misconfiguration - - -ast-grep-essentials: true - -rule: - kind: boolean_literal - pattern: $LITERAL - follows: - regex: ^=$ - follows: - kind: member_access_expression - inside: - kind: assignment_expression - all: - - has: - kind: member_access_expression - nthChild: 1 - regex: \.Cookie$ - - has: - kind: identifier - nthChild: 2 - regex: ^HttpOnly$ - -constraints: - LITERAL: - regex: ^false$ - diff --git a/rules/java/security/use-of-md5-digest-utils-java.yml b/rules/java/security/use-of-md5-digest-utils-java.yml deleted file mode 100644 index 553bac8a..00000000 --- a/rules/java/security/use-of-md5-digest-utils-java.yml +++ /dev/null @@ -1,42 +0,0 @@ -id: use-of-md5-digest-utils-java -language: java -severity: warning -message: >- - 'Detected MD5 hash algorithm which is considered insecure. MD5 is not - collision resistant and is therefore not suitable as a cryptographic - signature. Use HMAC instead.' -note: >- - [CWE-328] Use of Weak Hash - [REFERENCES] - - https://owasp.org/Top10/A02_2021-Cryptographic_Failures - -ast-grep-essentials: true - -rule: - kind: identifier - regex: ^getMd5Digest$ - nthChild: 2 - precedes: - nthChild: 3 - kind: argument_list - not: - has: - nthChild: 1 - inside: - kind: method_invocation - nthChild: 1 - inside: - kind: method_invocation - all: - - has: - kind: identifier - nthChild: 2 - regex: ^digest$ - - has: - kind: argument_list - nthChild: 3 - - not: - has: - stopBy: end - kind: ERROR - diff --git a/tests/csharp/httponly-false-csharp-test.yml b/tests/csharp/httponly-false-csharp-test.yml deleted file mode 100644 index e29a7eab..00000000 --- a/tests/csharp/httponly-false-csharp-test.yml +++ /dev/null @@ -1,9 +0,0 @@ -id: httponly-false-csharp -valid: - - | - myHttpOnlyCookie.HttpOnly = true; - - | - options.Cookie.HttpOnly = true; -invalid: - - | - options.Cookie.HttpOnly = false; diff --git a/tests/java/use-of-md5-digest-utils-java-test.yml b/tests/java/use-of-md5-digest-utils-java-test.yml deleted file mode 100644 index 769a4b52..00000000 --- a/tests/java/use-of-md5-digest-utils-java-test.yml +++ /dev/null @@ -1,7 +0,0 @@ -id: use-of-md5-digest-utils-java -valid: - - | - byte[] hashValue = DigestUtils.getSha512Digest().digest(password.getBytes()); -invalid: - - | - byte[] hashValue = DigestUtils.getMd5Digest().digest(password.getBytes()); From f4ca70925e3f12ba921d67ef6766416f00dc7647 Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Mon, 31 Mar 2025 11:53:35 +0530 Subject: [PATCH 5/7] file-access-before-action-c --- .../security/file-access-before-action-c.yml | 196 ++++++++++++++++++ .../file-access-before-action-c-snapshot.yml | 97 +++++++++ tests/c/file-access-before-action-c-test.yml | 23 ++ 3 files changed, 316 insertions(+) create mode 100644 rules/c/security/file-access-before-action-c.yml create mode 100644 tests/__snapshots__/file-access-before-action-c-snapshot.yml create mode 100644 tests/c/file-access-before-action-c-test.yml diff --git a/rules/c/security/file-access-before-action-c.yml b/rules/c/security/file-access-before-action-c.yml new file mode 100644 index 00000000..b01b9ac2 --- /dev/null +++ b/rules/c/security/file-access-before-action-c.yml @@ -0,0 +1,196 @@ +id: file-access-before-action-c +language: c +severity: warning +message: >- + A check is done with `access` and then the file is later used. There is no guarantee that the status of the file has not changed since the call to `access` which may allow attackers to bypass permission checks. +note: >- + [CWE-367]: Time-of-check Time-of-use (TOCTOU) Race Condition + [REFERENCES] + - https://wiki.sei.cmu.edu/confluence/display/c/FIO45-C.+Avoid+TOCTOU+race+conditions+while+accessing+files + +ast-grep-essentials: true + +utils: + PATTERN_1(identifier): + kind: identifier + regex: ^(fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir)$ + all: + - precedes: + kind: argument_list + has: + pattern: $SRC + - inside: + kind: call_expression + not: + inside: + stopBy: end + kind: parenthesized_expression + nthChild: 1 + inside: + stopBy: end + kind: if_statement + inside: + stopBy: end + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + kind: parenthesized_expression + has: + stopBy: end + any: + - kind: binary_expression + has: + stopBy: end + kind: parenthesized_expression + has: + kind: binary_expression + all: + - has: + kind: call_expression + nthChild: 1 + all: + - has: + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + precedes: + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + kind: identifier + nthChild: 2 + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + - kind: binary_expression + all: + - has: + nthChild: 1 + kind: call_expression + all: + - has: + nthChild: 1 + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + - has: + nthChild: 2 + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + nthChild: 2 + kind: identifier + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + nthChild: 2 + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + + identifier: + any: + - kind: identifier + regex: ^(fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir)$ + + PATTERN_3(field_expression): + kind: field_expression + has: + nthChild: 1 + stopBy: end + matches: identifier + all: + - precedes: + kind: argument_list + has: + pattern: $SRC + - inside: + kind: call_expression + not: + inside: + stopBy: end + kind: parenthesized_expression + inside: + stopBy: end + kind: if_statement + inside: + stopBy: end + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + kind: parenthesized_expression + has: + stopBy: end + any: + - kind: binary_expression + has: + stopBy: end + kind: parenthesized_expression + has: + kind: binary_expression + all: + - has: + kind: call_expression + nthChild: 1 + all: + - has: + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + precedes: + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + kind: identifier + nthChild: 2 + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + - kind: binary_expression + all: + - has: + nthChild: 1 + kind: call_expression + all: + - has: + nthChild: 1 + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + - has: + nthChild: 2 + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + nthChild: 2 + kind: identifier + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + nthChild: 2 + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + +rule: + any: + - matches: PATTERN_1(identifier) + - matches: PATTERN_3(field_expression) diff --git a/tests/__snapshots__/file-access-before-action-c-snapshot.yml b/tests/__snapshots__/file-access-before-action-c-snapshot.yml new file mode 100644 index 00000000..4a90516b --- /dev/null +++ b/tests/__snapshots__/file-access-before-action-c-snapshot.yml @@ -0,0 +1,97 @@ +id: file-access-before-action-c +snapshots: + ? | + { + const char *original_key = "path/to/file/filename"; + const char *mirror_key = "path/to/another/file/filename"; + + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + + void test_002(){ + const char *original_key = "path/to/file/filename"; + + if (access(original_key, W_OK) == 0){ + // ruleid: file-access-before-action + File *fp = fopen(original_key, "wb"); + } + } + : labels: + - source: unlink + style: primary + start: 250 + end: 256 + - source: original_key + style: secondary + start: 257 + end: 269 + - source: (original_key) + style: secondary + start: 256 + end: 270 + - source: original_key + style: secondary + start: 125 + end: 137 + - source: F_OK + style: secondary + start: 139 + end: 143 + - source: (original_key, F_OK) + style: secondary + start: 124 + end: 144 + - source: access + style: secondary + start: 118 + end: 124 + - source: access(original_key, F_OK) + style: secondary + start: 118 + end: 144 + - source: == + style: secondary + start: 145 + end: 147 + - source: '0' + style: secondary + start: 148 + end: 149 + - source: access(original_key, F_OK) == 0 + style: secondary + start: 118 + end: 149 + - source: (access(original_key, F_OK) == 0) + style: secondary + start: 117 + end: 150 + - source: (access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0) + style: secondary + start: 117 + end: 185 + - source: ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)) + style: secondary + start: 116 + end: 186 + - source: |- + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + style: secondary + start: 113 + end: 273 + - source: |- + { + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + style: secondary + start: 186 + end: 273 + - source: unlink(original_key) + style: secondary + start: 250 + end: 270 diff --git a/tests/c/file-access-before-action-c-test.yml b/tests/c/file-access-before-action-c-test.yml new file mode 100644 index 00000000..bad22733 --- /dev/null +++ b/tests/c/file-access-before-action-c-test.yml @@ -0,0 +1,23 @@ +id: file-access-before-action-c +valid: + - | + +invalid: + - | + { + const char *original_key = "path/to/file/filename"; + const char *mirror_key = "path/to/another/file/filename"; + + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + + void test_002(){ + const char *original_key = "path/to/file/filename"; + + if (access(original_key, W_OK) == 0){ + // ruleid: file-access-before-action + File *fp = fopen(original_key, "wb"); + } + } From c34b7044667c9c468f0fba3376d37872fd024e2b Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Mon, 31 Mar 2025 11:55:21 +0530 Subject: [PATCH 6/7] file-access-before-action-cpp --- .../file-access-before-action-cpp.yml | 273 ++++++++++++++++++ ...file-access-before-action-cpp-snapshot.yml | 95 ++++++ tests/c/file-access-before-action-c-test.yml | 1 - .../file-access-before-action-cpp-test.yml | 21 ++ 4 files changed, 389 insertions(+), 1 deletion(-) create mode 100644 rules/cpp/security/file-access-before-action-cpp.yml create mode 100644 tests/__snapshots__/file-access-before-action-cpp-snapshot.yml create mode 100644 tests/cpp/file-access-before-action-cpp-test.yml diff --git a/rules/cpp/security/file-access-before-action-cpp.yml b/rules/cpp/security/file-access-before-action-cpp.yml new file mode 100644 index 00000000..4e32d24f --- /dev/null +++ b/rules/cpp/security/file-access-before-action-cpp.yml @@ -0,0 +1,273 @@ +id: file-access-before-action-cpp +language: cpp +severity: warning +message: >- + A check is done with `access` and then the file is later used. There is no guarantee that the status of the file has not changed since the call to `access` which may allow attackers to bypass permission checks. +note: >- + [CWE-367]: Time-of-check Time-of-use (TOCTOU) Race Condition + [REFERENCES] + - https://wiki.sei.cmu.edu/confluence/display/c/FIO45-C.+Avoid+TOCTOU+race+conditions+while+accessing+files + +utils: + PATTERN_1(identifier): + kind: identifier + regex: ^(fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir)$ + all: + - precedes: + kind: argument_list + has: + pattern: $SRC + - inside: + kind: call_expression + not: + inside: + stopBy: end + kind: condition_clause + inside: + stopBy: end + kind: compound_statement + inside: + kind: if_statement + has: + kind: condition_clause + has: + stopBy: end + any: + - kind: binary_expression + has: + stopBy: end + kind: parenthesized_expression + has: + kind: binary_expression + all: + - has: + kind: call_expression + nthChild: 1 + all: + - has: + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + precedes: + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + kind: identifier + nthChild: 2 + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + - kind: binary_expression + all: + - has: + nthChild: 1 + kind: call_expression + all: + - has: + nthChild: 1 + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + - has: + nthChild: 2 + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + nthChild: 2 + kind: identifier + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + nthChild: 2 + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + + PATTERN_2(qualified_identifier): + kind: qualified_identifier + any: + - regex: ^(folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File)$ + - regex: ^(boost::)?(filesystem::file_size|filesystem::create_directory|filesystem::create_directories|filesystem::remove|filesystem::remove_all|filesystem::rename|filesystem::copy_file|filesystem::copy|filesystem::copy_directory|filesystem::resize_file|filesystem::last_write_time|filesystem::permissions|filesystem::symlink_status|filesystem::create_symlink|filesystem::create_hard_link|filesystem::read_symlink)$ + all: + - precedes: + kind: argument_list + has: + pattern: $SRC + - inside: + kind: call_expression + not: + inside: + stopBy: end + kind: condition_clause + inside: + stopBy: end + kind: compound_statement + inside: + kind: if_statement + has: + kind: condition_clause + has: + stopBy: end + any: + - kind: binary_expression + has: + stopBy: end + kind: parenthesized_expression + has: + kind: binary_expression + all: + - has: + kind: call_expression + nthChild: 1 + all: + - has: + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + precedes: + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + kind: identifier + nthChild: 2 + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + - kind: binary_expression + all: + - has: + nthChild: 1 + kind: call_expression + all: + - has: + nthChild: 1 + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + - has: + nthChild: 2 + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + nthChild: 2 + kind: identifier + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + nthChild: 2 + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + + identifier_and_qualified_identifier: + any: + - kind: identifier + regex: ^(fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir)$ + - kind: qualified_identifier + any: + - regex: ^(folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File)$ + - regex: ^(boost::)?(filesystem::file_size|filesystem::create_directory|filesystem::create_directories|filesystem::remove|filesystem::remove_all|filesystem::rename|filesystem::copy_file|filesystem::copy|filesystem::copy_directory|filesystem::resize_file|filesystem::last_write_time|filesystem::permissions|filesystem::symlink_status|filesystem::create_symlink|filesystem::create_hard_link|filesystem::read_symlink)$ + + PATTERN_3(field_expression): + kind: field_expression + has: + nthChild: 1 + stopBy: end + matches: identifier_and_qualified_identifier + all: + - precedes: + kind: argument_list + has: + pattern: $SRC + - inside: + kind: call_expression + not: + inside: + stopBy: end + kind: condition_clause + inside: + stopBy: end + kind: compound_statement + inside: + kind: if_statement + has: + kind: condition_clause + has: + stopBy: end + any: + - kind: binary_expression + has: + stopBy: end + kind: parenthesized_expression + has: + kind: binary_expression + all: + - has: + kind: call_expression + nthChild: 1 + all: + - has: + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + precedes: + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + kind: identifier + nthChild: 2 + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + - kind: binary_expression + all: + - has: + nthChild: 1 + kind: call_expression + all: + - has: + nthChild: 1 + kind: identifier + regex: ^(access|faccessat|faccessat2)$ + - has: + nthChild: 2 + kind: argument_list + all: + - has: + nthChild: 1 + pattern: $SRC + - has: + nthChild: 2 + kind: identifier + regex: ^(F_OK|R_OK|W_OK|X_OK)$ + - has: + nthChild: 2 + kind: number_literal + regex: ^(0)$ + follows: + regex: ^==$ + +rule: + any: + - matches: PATTERN_1(identifier) + - matches: PATTERN_2(qualified_identifier) + - matches: PATTERN_3(field_expression) diff --git a/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml b/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml new file mode 100644 index 00000000..62bd2ca1 --- /dev/null +++ b/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml @@ -0,0 +1,95 @@ +id: file-access-before-action-cpp +snapshots: + ? | + { + const char *original_key = "path/to/file/filename"; + const char *mirror_key = "path/to/another/file/filename"; + + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + + void test_002(){ + const char *original_key = "path/to/file/filename"; + if (access(original_key, W_OK) == 0){ + FILe *fp = fopen(original_key, "wb"); + } + } + : labels: + - source: unlink + style: primary + start: 250 + end: 256 + - source: original_key + style: secondary + start: 257 + end: 269 + - source: (original_key) + style: secondary + start: 256 + end: 270 + - source: original_key + style: secondary + start: 125 + end: 137 + - source: F_OK + style: secondary + start: 139 + end: 143 + - source: (original_key, F_OK) + style: secondary + start: 124 + end: 144 + - source: access + style: secondary + start: 118 + end: 124 + - source: access(original_key, F_OK) + style: secondary + start: 118 + end: 144 + - source: == + style: secondary + start: 145 + end: 147 + - source: '0' + style: secondary + start: 148 + end: 149 + - source: access(original_key, F_OK) == 0 + style: secondary + start: 118 + end: 149 + - source: (access(original_key, F_OK) == 0) + style: secondary + start: 117 + end: 150 + - source: (access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0) + style: secondary + start: 117 + end: 185 + - source: ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)) + style: secondary + start: 116 + end: 186 + - source: |- + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + style: secondary + start: 113 + end: 273 + - source: |- + { + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + style: secondary + start: 186 + end: 273 + - source: unlink(original_key) + style: secondary + start: 250 + end: 270 diff --git a/tests/c/file-access-before-action-c-test.yml b/tests/c/file-access-before-action-c-test.yml index bad22733..ef2e6093 100644 --- a/tests/c/file-access-before-action-c-test.yml +++ b/tests/c/file-access-before-action-c-test.yml @@ -17,7 +17,6 @@ invalid: const char *original_key = "path/to/file/filename"; if (access(original_key, W_OK) == 0){ - // ruleid: file-access-before-action File *fp = fopen(original_key, "wb"); } } diff --git a/tests/cpp/file-access-before-action-cpp-test.yml b/tests/cpp/file-access-before-action-cpp-test.yml new file mode 100644 index 00000000..cf479b38 --- /dev/null +++ b/tests/cpp/file-access-before-action-cpp-test.yml @@ -0,0 +1,21 @@ +id: file-access-before-action-cpp +valid: + - | + +invalid: + - | + { + const char *original_key = "path/to/file/filename"; + const char *mirror_key = "path/to/another/file/filename"; + + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + + void test_002(){ + const char *original_key = "path/to/file/filename"; + if (access(original_key, W_OK) == 0){ + FILe *fp = fopen(original_key, "wb"); + } + } From 195bdd37386476f378d265087e7ece248ac42ea0 Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Mon, 31 Mar 2025 12:04:14 +0530 Subject: [PATCH 7/7] Modified file-access-before-action-c and cpp --- .../file-access-before-action-cpp.yml | 2 + .../file-access-before-action-c-snapshot.yml | 95 +++++++++++++++++++ ...file-access-before-action-cpp-snapshot.yml | 94 ++++++++++++++++++ tests/c/file-access-before-action-c-test.yml | 21 ++-- .../file-access-before-action-cpp-test.yml | 21 ++-- 5 files changed, 213 insertions(+), 20 deletions(-) diff --git a/rules/cpp/security/file-access-before-action-cpp.yml b/rules/cpp/security/file-access-before-action-cpp.yml index 4e32d24f..20ccfc62 100644 --- a/rules/cpp/security/file-access-before-action-cpp.yml +++ b/rules/cpp/security/file-access-before-action-cpp.yml @@ -8,6 +8,8 @@ note: >- [REFERENCES] - https://wiki.sei.cmu.edu/confluence/display/c/FIO45-C.+Avoid+TOCTOU+race+conditions+while+accessing+files +ast-grep-essentials: true + utils: PATTERN_1(identifier): kind: identifier diff --git a/tests/__snapshots__/file-access-before-action-c-snapshot.yml b/tests/__snapshots__/file-access-before-action-c-snapshot.yml index 4a90516b..02e80626 100644 --- a/tests/__snapshots__/file-access-before-action-c-snapshot.yml +++ b/tests/__snapshots__/file-access-before-action-c-snapshot.yml @@ -1,5 +1,100 @@ id: file-access-before-action-c snapshots: + ? | + { + const char *original_key = "path/to/file/filename"; + const char *mirror_key = "path/to/another/file/filename"; + + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + + void test_002(){ + const char *original_key = "path/to/file/filename"; + + if (access(original_key, W_OK) == 0){ + File *fp = fopen(original_key, "wb"); + } + } + } + : labels: + - source: unlink + style: primary + start: 260 + end: 266 + - source: original_key + style: secondary + start: 267 + end: 279 + - source: (original_key) + style: secondary + start: 266 + end: 280 + - source: original_key + style: secondary + start: 131 + end: 143 + - source: F_OK + style: secondary + start: 145 + end: 149 + - source: (original_key, F_OK) + style: secondary + start: 130 + end: 150 + - source: access + style: secondary + start: 124 + end: 130 + - source: access(original_key, F_OK) + style: secondary + start: 124 + end: 150 + - source: == + style: secondary + start: 151 + end: 153 + - source: '0' + style: secondary + start: 154 + end: 155 + - source: access(original_key, F_OK) == 0 + style: secondary + start: 124 + end: 155 + - source: (access(original_key, F_OK) == 0) + style: secondary + start: 123 + end: 156 + - source: (access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0) + style: secondary + start: 123 + end: 191 + - source: ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)) + style: secondary + start: 122 + end: 192 + - source: |- + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + style: secondary + start: 119 + end: 285 + - source: |- + { + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + style: secondary + start: 192 + end: 285 + - source: unlink(original_key) + style: secondary + start: 260 + end: 280 ? | { const char *original_key = "path/to/file/filename"; diff --git a/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml b/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml index 62bd2ca1..34f76db2 100644 --- a/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml +++ b/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml @@ -1,5 +1,99 @@ id: file-access-before-action-cpp snapshots: + ? | + { + const char *original_key = "path/to/file/filename"; + const char *mirror_key = "path/to/another/file/filename"; + + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + + void test_002(){ + const char *original_key = "path/to/file/filename"; + if (access(original_key, W_OK) == 0){ + FILe *fp = fopen(original_key, "wb"); + } + } + } + : labels: + - source: unlink + style: primary + start: 260 + end: 266 + - source: original_key + style: secondary + start: 267 + end: 279 + - source: (original_key) + style: secondary + start: 266 + end: 280 + - source: original_key + style: secondary + start: 131 + end: 143 + - source: F_OK + style: secondary + start: 145 + end: 149 + - source: (original_key, F_OK) + style: secondary + start: 130 + end: 150 + - source: access + style: secondary + start: 124 + end: 130 + - source: access(original_key, F_OK) + style: secondary + start: 124 + end: 150 + - source: == + style: secondary + start: 151 + end: 153 + - source: '0' + style: secondary + start: 154 + end: 155 + - source: access(original_key, F_OK) == 0 + style: secondary + start: 124 + end: 155 + - source: (access(original_key, F_OK) == 0) + style: secondary + start: 123 + end: 156 + - source: (access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0) + style: secondary + start: 123 + end: 191 + - source: ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)) + style: secondary + start: 122 + end: 192 + - source: |- + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + style: secondary + start: 119 + end: 285 + - source: |- + { + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } + style: secondary + start: 192 + end: 285 + - source: unlink(original_key) + style: secondary + start: 260 + end: 280 ? | { const char *original_key = "path/to/file/filename"; diff --git a/tests/c/file-access-before-action-c-test.yml b/tests/c/file-access-before-action-c-test.yml index ef2e6093..b705bf0f 100644 --- a/tests/c/file-access-before-action-c-test.yml +++ b/tests/c/file-access-before-action-c-test.yml @@ -5,18 +5,19 @@ valid: invalid: - | { - const char *original_key = "path/to/file/filename"; - const char *mirror_key = "path/to/another/file/filename"; + const char *original_key = "path/to/file/filename"; + const char *mirror_key = "path/to/another/file/filename"; - if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ - copy_file("/bin/cp %s %s", original_key, mirror_key); - unlink(original_key); - } + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } - void test_002(){ - const char *original_key = "path/to/file/filename"; + void test_002(){ + const char *original_key = "path/to/file/filename"; - if (access(original_key, W_OK) == 0){ - File *fp = fopen(original_key, "wb"); + if (access(original_key, W_OK) == 0){ + File *fp = fopen(original_key, "wb"); + } } } diff --git a/tests/cpp/file-access-before-action-cpp-test.yml b/tests/cpp/file-access-before-action-cpp-test.yml index cf479b38..82424c6a 100644 --- a/tests/cpp/file-access-before-action-cpp-test.yml +++ b/tests/cpp/file-access-before-action-cpp-test.yml @@ -5,17 +5,18 @@ valid: invalid: - | { - const char *original_key = "path/to/file/filename"; - const char *mirror_key = "path/to/another/file/filename"; + const char *original_key = "path/to/file/filename"; + const char *mirror_key = "path/to/another/file/filename"; - if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ - copy_file("/bin/cp %s %s", original_key, mirror_key); - unlink(original_key); - } + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){ + copy_file("/bin/cp %s %s", original_key, mirror_key); + unlink(original_key); + } - void test_002(){ - const char *original_key = "path/to/file/filename"; - if (access(original_key, W_OK) == 0){ - FILe *fp = fopen(original_key, "wb"); + void test_002(){ + const char *original_key = "path/to/file/filename"; + if (access(original_key, W_OK) == 0){ + FILe *fp = fopen(original_key, "wb"); + } } }