From 2fc62d506425acafbe7c9c33a826fe52283dd3b2 Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Mon, 14 Oct 2024 11:12:17 +0530 Subject: [PATCH 1/3] file-access-before-action-cpp --- rules/cpp/file-access-before-action-cpp.yml | 78 ++++++++++++++++++ ...file-access-before-action-cpp-snapshot.yml | 79 +++++++++++++++++++ .../file-access-before-action-cpp-test.yml | 28 +++++++ 3 files changed, 185 insertions(+) create mode 100644 rules/cpp/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/file-access-before-action-cpp.yml b/rules/cpp/file-access-before-action-cpp.yml new file mode 100644 index 00000000..c29b83b0 --- /dev/null +++ b/rules/cpp/file-access-before-action-cpp.yml @@ -0,0 +1,78 @@ +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: + match_unlink_identifier: + kind: identifier + regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File + inside: + kind: call_expression + inside: + kind: expression_statement + inside: + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + kind: identifier + pattern: $R + - has: + kind: argument_list + all: + - has: + kind: identifier + regex: ^original_key + - has: + kind: identifier + regex: F_OK|R_OK|W_OK|X_OK + + match_fopen_identifier: + kind: identifier + regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File + inside: + kind: call_expression + inside: + stopBy: end + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + kind: identifier + pattern: $L + - has: + kind: argument_list + all: + - has: + kind: identifier + regex: ^original_key + - has: + kind: identifier + regex: F_OK|R_OK|W_OK|X_OK + +rule: + any: + - matches: match_unlink_identifier + - matches: match_fopen_identifier +constraints: + R: + regex: ^(access|faccessat|faccessat2|)$ + L: + regex: ^(access|faccessat|faccessat2|)$ 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..0c9cd833 --- /dev/null +++ b/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml @@ -0,0 +1,79 @@ +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); + + // ruleid: file-access-before-action + 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: 293 + end: 299 + - source: access + style: secondary + start: 118 + end: 124 + - 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(original_key, F_OK) + style: secondary + start: 118 + end: 144 + - source: |- + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)) + { + copy_file("/bin/cp %s %s", original_key, mirror_key); + + // ruleid: file-access-before-action + unlink(original_key); + } + style: secondary + start: 113 + end: 316 + - source: |- + { + copy_file("/bin/cp %s %s", original_key, mirror_key); + + // ruleid: file-access-before-action + unlink(original_key); + } + style: secondary + start: 187 + end: 316 + - source: unlink(original_key); + style: secondary + start: 293 + end: 314 + - source: unlink(original_key) + style: secondary + start: 293 + end: 313 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..fb725b2c --- /dev/null +++ b/tests/cpp/file-access-before-action-cpp-test.yml @@ -0,0 +1,28 @@ +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); + + // ruleid: file-access-before-action + 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 44cce441bb780123f005d19a8f35d4416f39f3e4 Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Mon, 14 Oct 2024 11:13:18 +0530 Subject: [PATCH 2/3] file-access-before-action-c --- .../security/file-access-before-action-c.yml | 78 ++++++++++++++++++ .../file-access-before-action-c-snapshot.yml | 79 +++++++++++++++++++ tests/c/file-access-before-action-c-test.yml | 28 +++++++ 3 files changed, 185 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..5d6498ab --- /dev/null +++ b/rules/c/security/file-access-before-action-c.yml @@ -0,0 +1,78 @@ +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 +utils: + match_unlink_identifier: + kind: identifier + regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File + inside: + kind: call_expression + inside: + kind: expression_statement + inside: + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + kind: identifier + pattern: $R + - has: + kind: argument_list + all: + - has: + kind: identifier + regex: ^original_key + - has: + kind: identifier + regex: F_OK|R_OK|W_OK|X_OK + + match_fopen_identifier: + kind: identifier + regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File + inside: + kind: call_expression + inside: + stopBy: end + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + kind: identifier + pattern: $L + - has: + kind: argument_list + all: + - has: + kind: identifier + regex: ^original_key + - has: + kind: identifier + regex: F_OK|R_OK|W_OK|X_OK + +rule: + any: + - matches: match_unlink_identifier + - matches: match_fopen_identifier +constraints: + R: + regex: ^(access|faccessat|faccessat2|)$ + L: + regex: ^(access|faccessat|faccessat2|)$ 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..184a6e9d --- /dev/null +++ b/tests/__snapshots__/file-access-before-action-c-snapshot.yml @@ -0,0 +1,79 @@ +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); + + // ruleid: file-access-before-action + 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: 293 + end: 299 + - source: access + style: secondary + start: 118 + end: 124 + - 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(original_key, F_OK) + style: secondary + start: 118 + end: 144 + - source: |- + if ((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)) + { + copy_file("/bin/cp %s %s", original_key, mirror_key); + + // ruleid: file-access-before-action + unlink(original_key); + } + style: secondary + start: 113 + end: 316 + - source: |- + { + copy_file("/bin/cp %s %s", original_key, mirror_key); + + // ruleid: file-access-before-action + unlink(original_key); + } + style: secondary + start: 187 + end: 316 + - source: unlink(original_key); + style: secondary + start: 293 + end: 314 + - source: unlink(original_key) + style: secondary + start: 293 + end: 313 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..0135ed02 --- /dev/null +++ b/tests/c/file-access-before-action-c-test.yml @@ -0,0 +1,28 @@ +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); + + // ruleid: file-access-before-action + 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 5f1a32a441d884221f7c0366b498e5032621eb10 Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Tue, 15 Oct 2024 18:08:20 +0530 Subject: [PATCH 3/3] changes in file-access-before-action inc/cpp --- .../security/file-access-before-action-c.yml | 111 ++++++++++-------- rules/cpp/file-access-before-action-cpp.yml | 111 ++++++++++-------- .../file-access-before-action-c-snapshot.yml | 8 ++ ...file-access-before-action-cpp-snapshot.yml | 8 ++ 4 files changed, 142 insertions(+), 96 deletions(-) diff --git a/rules/c/security/file-access-before-action-c.yml b/rules/c/security/file-access-before-action-c.yml index 5d6498ab..b12a0eb3 100644 --- a/rules/c/security/file-access-before-action-c.yml +++ b/rules/c/security/file-access-before-action-c.yml @@ -13,60 +13,75 @@ utils: match_unlink_identifier: kind: identifier regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File - inside: - kind: call_expression - inside: - kind: expression_statement - inside: - kind: compound_statement + all: + - inside: + kind: call_expression inside: - stopBy: end - kind: if_statement - has: - stopBy: end - kind: call_expression - all: - - has: - kind: identifier - pattern: $R - - has: - kind: argument_list - all: - - has: - kind: identifier - regex: ^original_key - - has: - kind: identifier - regex: F_OK|R_OK|W_OK|X_OK - - match_fopen_identifier: - kind: identifier - regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File - inside: - kind: call_expression - inside: - stopBy: end - kind: compound_statement - inside: - stopBy: end - kind: if_statement - has: - stopBy: end - kind: call_expression - all: - - has: - kind: identifier - pattern: $L - - has: - kind: argument_list + kind: expression_statement + inside: + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + stopBy: end + kind: call_expression all: - has: kind: identifier - regex: ^original_key + pattern: $R - has: - kind: identifier - regex: F_OK|R_OK|W_OK|X_OK + kind: argument_list + all: + - has: + kind: identifier + pattern: $O + - has: + kind: identifier + regex: F_OK|R_OK|W_OK|X_OK + - precedes: + stopBy: neighbor + kind: argument_list + has: + stopBy: neighbor + kind: identifier + pattern: $O + match_fopen_identifier: + kind: identifier + regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File + all: + - inside: + kind: call_expression + inside: + stopBy: end + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + kind: identifier + pattern: $L + - has: + kind: argument_list + all: + - has: + kind: identifier + pattern: $O + - has: + kind: identifier + regex: F_OK|R_OK|W_OK|X_OK + - precedes: + stopBy: neighbor + kind: argument_list + has: + stopBy: neighbor + kind: identifier + pattern: $O rule: any: - matches: match_unlink_identifier diff --git a/rules/cpp/file-access-before-action-cpp.yml b/rules/cpp/file-access-before-action-cpp.yml index c29b83b0..b6fe3efd 100644 --- a/rules/cpp/file-access-before-action-cpp.yml +++ b/rules/cpp/file-access-before-action-cpp.yml @@ -13,60 +13,75 @@ utils: match_unlink_identifier: kind: identifier regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File - inside: - kind: call_expression - inside: - kind: expression_statement - inside: - kind: compound_statement + all: + - inside: + kind: call_expression inside: - stopBy: end - kind: if_statement - has: - stopBy: end - kind: call_expression - all: - - has: - kind: identifier - pattern: $R - - has: - kind: argument_list - all: - - has: - kind: identifier - regex: ^original_key - - has: - kind: identifier - regex: F_OK|R_OK|W_OK|X_OK - - match_fopen_identifier: - kind: identifier - regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File - inside: - kind: call_expression - inside: - stopBy: end - kind: compound_statement - inside: - stopBy: end - kind: if_statement - has: - stopBy: end - kind: call_expression - all: - - has: - kind: identifier - pattern: $L - - has: - kind: argument_list + kind: expression_statement + inside: + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + stopBy: end + kind: call_expression all: - has: kind: identifier - regex: ^original_key + pattern: $R - has: - kind: identifier - regex: F_OK|R_OK|W_OK|X_OK + kind: argument_list + all: + - has: + kind: identifier + pattern: $O + - has: + kind: identifier + regex: F_OK|R_OK|W_OK|X_OK + - precedes: + stopBy: neighbor + kind: argument_list + has: + stopBy: neighbor + kind: identifier + pattern: $O + match_fopen_identifier: + kind: identifier + regex: unlink|fopen|freopen|remove|rename|access|open|stat|lstat|unlink|mkdir|rmdir|chdir|folly::readFile|folly::writeFile|folly::writeFileAtomic|folly::writeFileAtomicNoThrow|folly::File + all: + - inside: + kind: call_expression + inside: + stopBy: end + kind: compound_statement + inside: + stopBy: end + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + kind: identifier + pattern: $L + - has: + kind: argument_list + all: + - has: + kind: identifier + pattern: $O + - has: + kind: identifier + regex: F_OK|R_OK|W_OK|X_OK + - precedes: + stopBy: neighbor + kind: argument_list + has: + stopBy: neighbor + kind: identifier + pattern: $O rule: any: - matches: match_unlink_identifier diff --git a/tests/__snapshots__/file-access-before-action-c-snapshot.yml b/tests/__snapshots__/file-access-before-action-c-snapshot.yml index 184a6e9d..42134589 100644 --- a/tests/__snapshots__/file-access-before-action-c-snapshot.yml +++ b/tests/__snapshots__/file-access-before-action-c-snapshot.yml @@ -77,3 +77,11 @@ snapshots: style: secondary start: 293 end: 313 + - source: original_key + style: secondary + start: 300 + end: 312 + - source: (original_key) + style: secondary + start: 299 + end: 313 diff --git a/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml b/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml index 0c9cd833..f5235ac0 100644 --- a/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml +++ b/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml @@ -77,3 +77,11 @@ snapshots: style: secondary start: 293 end: 313 + - source: original_key + style: secondary + start: 300 + end: 312 + - source: (original_key) + style: secondary + start: 299 + end: 313