From 838c1db47e81ab568b74da50cd43c38814b3855f Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Tue, 15 Oct 2024 18:10:51 +0530 Subject: [PATCH 1/3] changes in file-access-before-action in cpp --- .../c/security/file-stat-before-action-c.yml | 82 ++++++++++ .../security/file-stat-before-action-cpp.yml | 82 ++++++++++ .../file-stat-before-action-c-snapshot.yml | 150 ++++++++++++++++++ tests/c/file-stat-before-action-c-test.yml | 43 +++++ .../cpp/file-stat-before-action-cpp-test.yml | 43 +++++ 5 files changed, 400 insertions(+) create mode 100644 rules/c/security/file-stat-before-action-c.yml create mode 100644 rules/cpp/security/file-stat-before-action-cpp.yml create mode 100644 tests/__snapshots__/file-stat-before-action-c-snapshot.yml create mode 100644 tests/c/file-stat-before-action-c-test.yml create mode 100644 tests/cpp/file-stat-before-action-cpp-test.yml 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..f026f23f --- /dev/null +++ b/tests/__snapshots__/file-stat-before-action-c-snapshot.yml @@ -0,0 +1,150 @@ +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 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..76bed212 --- /dev/null +++ b/tests/c/file-stat-before-action-c-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); + } 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); + } From 494c0a6e8d900a3a30fb16a9e1493bed2f5f08e3 Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Wed, 16 Oct 2024 10:25:45 +0530 Subject: [PATCH 2/3] file-stat-before-action-c/cpp --- tests/c/file-stat-before-action-c-test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/c/file-stat-before-action-c-test.yml b/tests/c/file-stat-before-action-c-test.yml index 76bed212..24f2c4c8 100644 --- a/tests/c/file-stat-before-action-c-test.yml +++ b/tests/c/file-stat-before-action-c-test.yml @@ -18,7 +18,6 @@ invalid: throw message; } - // Read the file MvString s, ss; while (fgets(data, sizeof(data), fp) != (char *)0) { From 7abb5d50fcd899cc890b26b34ed7df68b50a0f03 Mon Sep 17 00:00:00 2001 From: ESS-ENN Date: Wed, 16 Oct 2024 10:28:09 +0530 Subject: [PATCH 3/3] changes in file-stat-before-action-c/cpp --- .../file-stat-before-action-c-snapshot.yml | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/tests/__snapshots__/file-stat-before-action-c-snapshot.yml b/tests/__snapshots__/file-stat-before-action-c-snapshot.yml index f026f23f..d5a64b98 100644 --- a/tests/__snapshots__/file-stat-before-action-c-snapshot.yml +++ b/tests/__snapshots__/file-stat-before-action-c-snapshot.yml @@ -148,3 +148,148 @@ snapshots: 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