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 927df22a87bf50b4b2f23b59a43804c0289f605b Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Tue, 15 Oct 2024 18:43:01 +0530 Subject: [PATCH 3/3] changes in file-access-before-action-c/cpp --- tests/c/file-access-before-action-c-test.yml | 117 ++++++++++++++---- .../file-access-before-action-cpp-test.yml | 117 ++++++++++++++---- 2 files changed, 182 insertions(+), 52 deletions(-) diff --git a/tests/c/file-access-before-action-c-test.yml b/tests/c/file-access-before-action-c-test.yml index 0135ed02..b12a0eb3 100644 --- a/tests/c/file-access-before-action-c-test.yml +++ b/tests/c/file-access-before-action-c-test.yml @@ -1,28 +1,93 @@ id: file-access-before-action-c -valid: - - | +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 + all: + - 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 + 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 -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"); - } - } + 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 + - matches: match_fopen_identifier +constraints: + R: + regex: ^(access|faccessat|faccessat2|)$ + L: + regex: ^(access|faccessat|faccessat2|)$ diff --git a/tests/cpp/file-access-before-action-cpp-test.yml b/tests/cpp/file-access-before-action-cpp-test.yml index fb725b2c..b6fe3efd 100644 --- a/tests/cpp/file-access-before-action-cpp-test.yml +++ b/tests/cpp/file-access-before-action-cpp-test.yml @@ -1,28 +1,93 @@ id: file-access-before-action-cpp -valid: - - | +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 + all: + - 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 + 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 -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"); - } - } + 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 + - matches: match_fopen_identifier +constraints: + R: + regex: ^(access|faccessat|faccessat2|)$ + L: + regex: ^(access|faccessat|faccessat2|)$