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/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..20ccfc62 --- /dev/null +++ b/rules/cpp/security/file-access-before-action-cpp.yml @@ -0,0 +1,275 @@ +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 + +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: 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-c-snapshot.yml b/tests/__snapshots__/file-access-before-action-c-snapshot.yml new file mode 100644 index 00000000..02e80626 --- /dev/null +++ b/tests/__snapshots__/file-access-before-action-c-snapshot.yml @@ -0,0 +1,192 @@ +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"; + 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/__snapshots__/file-access-before-action-cpp-snapshot.yml b/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml new file mode 100644 index 00000000..34f76db2 --- /dev/null +++ b/tests/__snapshots__/file-access-before-action-cpp-snapshot.yml @@ -0,0 +1,189 @@ +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"; + 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 new file mode 100644 index 00000000..b705bf0f --- /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){ + 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..82424c6a --- /dev/null +++ b/tests/cpp/file-access-before-action-cpp-test.yml @@ -0,0 +1,22 @@ +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"); + } + } + }