diff --git a/rules/c/security/file-stat-before-action-c.yml b/rules/c/security/file-stat-before-action-c.yml new file mode 100644 index 00000000..9a612fe1 --- /dev/null +++ b/rules/c/security/file-stat-before-action-c.yml @@ -0,0 +1,82 @@ +id: file-stat-before-action-c +language: c +severity: warning +message: >- + A check is done with `stat` and then the file is used. There is no + guarantee that the status of the file has not changed since the call to + `stat` 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_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: expression_statement + inside: + kind: compound_statement + inside: + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + kind: identifier + regex: ^(fstatat|_fstatat)$ + - has: + stopBy: neighbor + kind: argument_list + all: + - has: + stopBy: neighbor + kind: identifier + - has: + stopBy: neighbor + kind: call_expression + all: + - has: + stopBy: neighbor + kind: field_expression + - has: + stopBy: neighbor + kind: argument_list + + match_fopen_identifier_2: + 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: expression_statement + inside: + kind: compound_statement + inside: + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + stopBy: neighbor + kind: identifier + regex: "^stat|_stat|lstat|_lstat$" + - has: + stopBy: neighbor + kind: argument_list + has: + stopBy: neighbor + kind: call_expression + +rule: + any: + - matches: match_fopen_identifier + - matches: match_fopen_identifier_2 diff --git a/rules/cpp/security/file-stat-before-action-cpp.yml b/rules/cpp/security/file-stat-before-action-cpp.yml new file mode 100644 index 00000000..cab3931b --- /dev/null +++ b/rules/cpp/security/file-stat-before-action-cpp.yml @@ -0,0 +1,82 @@ +id: file-stat-before-action-cpp +language: cpp +severity: warning +message: >- + A check is done with `stat` and then the file is used. There is no + guarantee that the status of the file has not changed since the call to + `stat` 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_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: expression_statement + inside: + kind: compound_statement + inside: + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + kind: identifier + regex: ^(fstatat|_fstatat)$ + - has: + stopBy: neighbor + kind: argument_list + all: + - has: + stopBy: neighbor + kind: identifier + - has: + stopBy: neighbor + kind: call_expression + all: + - has: + stopBy: neighbor + kind: field_expression + - has: + stopBy: neighbor + kind: argument_list + + match_fopen_identifier_2: + 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: expression_statement + inside: + kind: compound_statement + inside: + kind: if_statement + has: + stopBy: end + kind: call_expression + all: + - has: + stopBy: neighbor + kind: identifier + regex: "^stat|_stat|lstat|_lstat$" + - has: + stopBy: neighbor + kind: argument_list + has: + stopBy: neighbor + kind: call_expression + +rule: + any: + - matches: match_fopen_identifier + - matches: match_fopen_identifier_2 diff --git a/tests/__snapshots__/file-stat-before-action-c-snapshot.yml b/tests/__snapshots__/file-stat-before-action-c-snapshot.yml new file mode 100644 index 00000000..d5a64b98 --- /dev/null +++ b/tests/__snapshots__/file-stat-before-action-c-snapshot.yml @@ -0,0 +1,295 @@ +id: file-stat-before-action-c +snapshots: + ? | + if (stat(file.c_str(), &buf) == 0) + { + + // Open the file for reading + // ruleid: file-stat-before-action + fp = fopen(file.c_str(), "r"); + if (fp == NULL) + { + char message[2560]; + sprintf(message, "File '%s' Cound Not be Opened", file.c_str()); + // DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" ); + throw message; + } + + // Read the file + MvString s, ss; + while (fgets(data, sizeof(data), fp) != (char *)0) + { + s = data; + s.trimBoth(); + if (s.compare(0, 5, "GROUP") == 0) + { + // size_t t = s.find_last_of( ":" ); + size_t t = s.find(":"); + if (t != string::npos) + { + ss = s.substr(t + 1).c_str(); + ss.trimBoth(); + ss = ss.substr(1, ss.length() - 3).c_str(); + group_list.push_back(ss); + } + } + } + + // Close the file + fclose(fp); + } + : labels: + - source: fopen + style: primary + start: 123 + end: 128 + - source: stat + style: secondary + start: 4 + end: 8 + - source: file.c_str() + style: secondary + start: 9 + end: 21 + - source: (file.c_str(), &buf) + style: secondary + start: 8 + end: 28 + - source: stat(file.c_str(), &buf) + style: secondary + start: 4 + end: 28 + - source: |- + if (stat(file.c_str(), &buf) == 0) + { + + // Open the file for reading + // ruleid: file-stat-before-action + fp = fopen(file.c_str(), "r"); + if (fp == NULL) + { + char message[2560]; + sprintf(message, "File '%s' Cound Not be Opened", file.c_str()); + // DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" ); + throw message; + } + + // Read the file + MvString s, ss; + while (fgets(data, sizeof(data), fp) != (char *)0) + { + s = data; + s.trimBoth(); + if (s.compare(0, 5, "GROUP") == 0) + { + // size_t t = s.find_last_of( ":" ); + size_t t = s.find(":"); + if (t != string::npos) + { + ss = s.substr(t + 1).c_str(); + ss.trimBoth(); + ss = ss.substr(1, ss.length() - 3).c_str(); + group_list.push_back(ss); + } + } + } + + // Close the file + fclose(fp); + } + style: secondary + start: 0 + end: 989 + - source: |- + { + + // Open the file for reading + // ruleid: file-stat-before-action + fp = fopen(file.c_str(), "r"); + if (fp == NULL) + { + char message[2560]; + sprintf(message, "File '%s' Cound Not be Opened", file.c_str()); + // DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" ); + throw message; + } + + // Read the file + MvString s, ss; + while (fgets(data, sizeof(data), fp) != (char *)0) + { + s = data; + s.trimBoth(); + if (s.compare(0, 5, "GROUP") == 0) + { + // size_t t = s.find_last_of( ":" ); + size_t t = s.find(":"); + if (t != string::npos) + { + ss = s.substr(t + 1).c_str(); + ss.trimBoth(); + ss = ss.substr(1, ss.length() - 3).c_str(); + group_list.push_back(ss); + } + } + } + + // Close the file + fclose(fp); + } + style: secondary + start: 36 + end: 989 + - source: fp = fopen(file.c_str(), "r"); + style: secondary + start: 118 + end: 148 + - source: fopen(file.c_str(), "r") + style: secondary + start: 123 + end: 147 + ? | + if (stat(file.c_str(), &buf) == 0) + { + + // Open the file for reading + // ruleid: file-stat-before-action + fp = fopen(file.c_str(), "r"); + if (fp == NULL) + { + char message[2560]; + sprintf(message, "File '%s' Cound Not be Opened", file.c_str()); + // DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" ); + throw message; + } + + MvString s, ss; + while (fgets(data, sizeof(data), fp) != (char *)0) + { + s = data; + s.trimBoth(); + if (s.compare(0, 5, "GROUP") == 0) + { + // size_t t = s.find_last_of( ":" ); + size_t t = s.find(":"); + if (t != string::npos) + { + ss = s.substr(t + 1).c_str(); + ss.trimBoth(); + ss = ss.substr(1, ss.length() - 3).c_str(); + group_list.push_back(ss); + } + } + } + + // Close the file + fclose(fp); + } + : labels: + - source: fopen + style: primary + start: 123 + end: 128 + - source: stat + style: secondary + start: 4 + end: 8 + - source: file.c_str() + style: secondary + start: 9 + end: 21 + - source: (file.c_str(), &buf) + style: secondary + start: 8 + end: 28 + - source: stat(file.c_str(), &buf) + style: secondary + start: 4 + end: 28 + - source: |- + if (stat(file.c_str(), &buf) == 0) + { + + // Open the file for reading + // ruleid: file-stat-before-action + fp = fopen(file.c_str(), "r"); + if (fp == NULL) + { + char message[2560]; + sprintf(message, "File '%s' Cound Not be Opened", file.c_str()); + // DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" ); + throw message; + } + + MvString s, ss; + while (fgets(data, sizeof(data), fp) != (char *)0) + { + s = data; + s.trimBoth(); + if (s.compare(0, 5, "GROUP") == 0) + { + // size_t t = s.find_last_of( ":" ); + size_t t = s.find(":"); + if (t != string::npos) + { + ss = s.substr(t + 1).c_str(); + ss.trimBoth(); + ss = ss.substr(1, ss.length() - 3).c_str(); + group_list.push_back(ss); + } + } + } + + // Close the file + fclose(fp); + } + style: secondary + start: 0 + end: 967 + - source: |- + { + + // Open the file for reading + // ruleid: file-stat-before-action + fp = fopen(file.c_str(), "r"); + if (fp == NULL) + { + char message[2560]; + sprintf(message, "File '%s' Cound Not be Opened", file.c_str()); + // DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" ); + throw message; + } + + MvString s, ss; + while (fgets(data, sizeof(data), fp) != (char *)0) + { + s = data; + s.trimBoth(); + if (s.compare(0, 5, "GROUP") == 0) + { + // size_t t = s.find_last_of( ":" ); + size_t t = s.find(":"); + if (t != string::npos) + { + ss = s.substr(t + 1).c_str(); + ss.trimBoth(); + ss = ss.substr(1, ss.length() - 3).c_str(); + group_list.push_back(ss); + } + } + } + + // Close the file + fclose(fp); + } + style: secondary + start: 36 + end: 967 + - source: fp = fopen(file.c_str(), "r"); + style: secondary + start: 118 + end: 148 + - source: fopen(file.c_str(), "r") + style: secondary + start: 123 + end: 147 diff --git a/tests/c/file-stat-before-action-c-test.yml b/tests/c/file-stat-before-action-c-test.yml new file mode 100644 index 00000000..24f2c4c8 --- /dev/null +++ b/tests/c/file-stat-before-action-c-test.yml @@ -0,0 +1,42 @@ +id: file-stat-before-action-c +valid: + - | + +invalid: + - | + if (stat(file.c_str(), &buf) == 0) + { + + // Open the file for reading + // ruleid: file-stat-before-action + fp = fopen(file.c_str(), "r"); + if (fp == NULL) + { + char message[2560]; + sprintf(message, "File '%s' Cound Not be Opened", file.c_str()); + // DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" ); + throw message; + } + + MvString s, ss; + while (fgets(data, sizeof(data), fp) != (char *)0) + { + s = data; + s.trimBoth(); + if (s.compare(0, 5, "GROUP") == 0) + { + // size_t t = s.find_last_of( ":" ); + size_t t = s.find(":"); + if (t != string::npos) + { + ss = s.substr(t + 1).c_str(); + ss.trimBoth(); + ss = ss.substr(1, ss.length() - 3).c_str(); + group_list.push_back(ss); + } + } + } + + // Close the file + fclose(fp); + } diff --git a/tests/cpp/file-stat-before-action-cpp-test.yml b/tests/cpp/file-stat-before-action-cpp-test.yml new file mode 100644 index 00000000..76bed212 --- /dev/null +++ b/tests/cpp/file-stat-before-action-cpp-test.yml @@ -0,0 +1,43 @@ +id: file-stat-before-action-c +valid: + - | + +invalid: + - | + if (stat(file.c_str(), &buf) == 0) + { + + // Open the file for reading + // ruleid: file-stat-before-action + fp = fopen(file.c_str(), "r"); + if (fp == NULL) + { + char message[2560]; + sprintf(message, "File '%s' Cound Not be Opened", file.c_str()); + // DISPLAY_MSG_ERROR( this, message, "GetFileContents", "System" ); + throw message; + } + + // Read the file + MvString s, ss; + while (fgets(data, sizeof(data), fp) != (char *)0) + { + s = data; + s.trimBoth(); + if (s.compare(0, 5, "GROUP") == 0) + { + // size_t t = s.find_last_of( ":" ); + size_t t = s.find(":"); + if (t != string::npos) + { + ss = s.substr(t + 1).c_str(); + ss.trimBoth(); + ss = ss.substr(1, ss.length() - 3).c_str(); + group_list.push_back(ss); + } + } + } + + // Close the file + fclose(fp); + }