diff --git a/rules/c/security/return-c-str-c.yml b/rules/c/security/return-c-str-c.yml deleted file mode 100644 index b6f2ea92..00000000 --- a/rules/c/security/return-c-str-c.yml +++ /dev/null @@ -1,27 +0,0 @@ -id: return-c-str-cpp -language: cpp -severity: warning -message: >- - "`$FUNC` returns a pointer to the memory owned by `$STR`. This pointer - is invalid after `$STR` goes out of scope, which can trigger a use after - free." -note: >- - [CWE-416] Use After Free - [REFERENCES] - - https://wiki.sei.cmu.edu/confluence/display/c/DCL30-C.+Declare+objects+with+appropriate+storage+durations - - https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP54-CPP.+Do+not+access+an+object+outside+of+its+lifetime - -rule: - kind: return_statement - any: - - pattern: return basic_string<$TYPE>($$$).$METHOD(); - - pattern: return std::basic_string<$TYPE>($$$).$METHOD(); - - pattern: return string($$$).$METHOD(); - - pattern: return std::string($$$).$METHOD(); - - pattern: return wstring($$$).$METHOD(); - - pattern: return std::wstring($$$).$METHOD(); - -constraints: - METHOD: - regex: ^(c_str|data)$ - diff --git a/rules/cpp/security/return-c-str-cpp.yml b/rules/cpp/security/return-c-str-cpp.yml new file mode 100644 index 00000000..7637bdcf --- /dev/null +++ b/rules/cpp/security/return-c-str-cpp.yml @@ -0,0 +1,124 @@ +id: return-c-str-cpp +language: cpp +severity: warning +message: >- + "`$FUNC` returns a pointer to the memory owned by `$STR`. This pointer + is invalid after `$STR` goes out of scope, which can trigger a use after + free." +note: >- + [CWE-416] Use After Free + [REFERENCES] + - https://wiki.sei.cmu.edu/confluence/display/c/DCL30-C.+Declare+objects+with+appropriate+storage+durations + - https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP54-CPP.+Do+not+access+an+object+outside+of+its+lifetime + +ast-grep-essentials: true + +rule: + any: + - pattern: return basic_string<$TYPE>($$$).$METHOD(); + - pattern: return std::basic_string<$TYPE>($$$).$METHOD(); + - pattern: return string($$$).$METHOD(); + - pattern: return std::string($$$).$METHOD(); + - pattern: return wstring($$$).$METHOD(); + - pattern: return std::wstring($$$).$METHOD(); + - pattern: return $STR.$METHOD(); + any: + - follows: + stopBy: end + all: + - not: + has: + stopBy: end + kind: storage_class_specifier + - any: + - kind: declaration + not: + pattern: $STR_VAL $STR = "$STRG"; + - has: + pattern: $STR_VAL + - has: + stopBy: end + pattern: $STR + - inside: + stopBy: end + follows: + stopBy: end + all: + - not: + has: + stopBy: end + kind: storage_class_specifier + - any: + - kind: declaration + not: + pattern: $STR_VAL $STR = "$STRG"; + - has: + pattern: $STR_VAL + - has: + pattern: $STR + - inside: + stopBy: end + follows: + stopBy: end + all: + - not: + has: + stopBy: end + kind: storage_class_specifier + - any: + - kind: pointer_declarator + not: + has: + stopBy: end + pattern: $STR_VAL $STR = "$STRG"; + has: + kind: function_declarator + all: + - has: + stopBy: end + any: + - kind: qualified_identifier + - kind: type_identifier + regex: ^(basic_string<.*>|std::basic_string<.*>|string|std::string|wstring|std::wstring|string(.*)|std::string(.*)|wstring(.*)|std::wstring(.*)|basic_string<.*>(.*)|std::basic_string<.*>(.*))$ + - has: + stopBy: end + pattern: $STR + - follows: + stopBy: end + all: + - not: + has: + stopBy: end + kind: storage_class_specifier + - any: + - kind: pointer_declarator + has: + kind: function_declarator + all: + - not: + has: + stopBy: end + pattern: $STR_VAL $STR = "$STRG"; + - has: + stopBy: end + any: + - kind: qualified_identifier + - kind: type_identifier + regex: ^(basic_string<.*>|std::basic_string<.*>|string|std::string|wstring|std::wstring|string(.*)|std::string(.*)|wstring(.*)|std::wstring(.*)|basic_string<.*>(.*)|std::basic_string<.*>(.*))$ + - has: + stopBy: end + pattern: $STR + - pattern: return $STR_VAL.$METHOD(); + not: + all: + - has: + stopBy: end + kind: ERROR + - inside: + stopBy: end + kind: ERROR +constraints: + METHOD: + regex: ^(c_str|data)$ + STR_VAL: + regex: ^(basic_string<.*>|std::basic_string<.*>|string|std::string|wstring|std::wstring|string(.*)|std::string(.*)|wstring(.*)|std::wstring(.*)|basic_string<.*>(.*)|std::basic_string<.*>(.*))$ diff --git a/rules/cpp/security/std-return-data-cpp.yml b/rules/cpp/security/std-return-data-cpp.yml new file mode 100644 index 00000000..3a2b0be6 --- /dev/null +++ b/rules/cpp/security/std-return-data-cpp.yml @@ -0,0 +1,85 @@ +id: std-return-data-cpp +language: cpp +severity: warning +message: >- + $FUNC` returns a pointer to the memory owned by `$VAR`. This pointer + is invalid after `$VAR` goes out of scope, which can trigger a use after + free. +note: >- + [CWE-416: Use After Free. + [REFERENCES] + - https://wiki.sei.cmu.edu/confluence/display/c/DCL30-C.+Declare+objects+with+appropriate+storage+durations + +ast-grep-essentials: true + +rule: + kind: return_statement + pattern: return $VAR.data(); + all: + - inside: + stopBy: end + kind: function_definition + all: + - has: + nthChild: 1 + pattern: $RETURN_TYPE + - has: + kind: pointer_declarator + - any: + - follows: + stopBy: end + all: + - has: + nthChild: 1 + regex: ^(array<.*>|std::array<.*>|deque<.*>|std::deque<.*>|forward_list<.*>|std::forward_list<.*>|list<.*>|std::list<.*>|map<.*, .*>|std::map<.*, .*>|multimap<.*, .*>|std::multimap<.*, .*>|multiset<.*>|std::multiset<.*>|set<.*>|std::set<.*>|unordered_map<.*>|std::unordered_map<.*>|unordered_multimap<.*, .*>|std::unordered_multimap<.*, .*>|unordered_multiset<.*>|std::unordered_multiset<.*>|unordered_set<.*>|std::unordered_set<.*>|vector<.*>|std::vector<.*>)$ + - has: + stopBy: end + # nthChild: 2 + pattern: $VAR + - not: + inside: + stopBy: end + has: + kind: storage_class_specifier + - inside: + stopBy: end + kind: compound_statement + - inside: + stopBy: end + follows: + stopBy: end + all: + - has: + nthChild: 1 + regex: ^(array<.*>|std::array<.*>|deque<.*>|std::deque<.*>|forward_list<.*>|std::forward_list<.*>|list<.*>|std::list<.*>|map<.*, .*>|std::map<.*, .*>|multimap<.*, .*>|std::multimap<.*, .*>|multiset<.*>|std::multiset<.*>|set<.*>|std::set<.*>|unordered_map<.*>|std::unordered_map<.*>|unordered_multimap<.*, .*>|std::unordered_multimap<.*, .*>|unordered_multiset<.*>|std::unordered_multiset<.*>|unordered_set<.*>|std::unordered_set<.*>|vector<.*>|std::vector<.*>)$ + - has: + # nthChild: 2 + stopBy: end + pattern: $VAR + - not: + inside: + stopBy: end + has: + kind: storage_class_specifier + - inside: + stopBy: end + kind: compound_statement + - inside: + stopBy: end + follows: + stopBy: end + kind: pointer_declarator + all: + - has: + stopBy: end + nthChild: 1 + regex: ^(array<.*>|std::array<.*>|deque<.*>|std::deque<.*>|forward_list<.*>|std::forward_list<.*>|list<.*>|std::list<.*>|map<.*, .*>|std::map<.*, .*>|multimap<.*, .*>|std::multimap<.*, .*>|multiset<.*>|std::multiset<.*>|set<.*>|std::set<.*>|unordered_map<.*>|std::unordered_map<.*>|unordered_multimap<.*, .*>|std::unordered_multimap<.*, .*>|unordered_multiset<.*>|std::unordered_multiset<.*>|unordered_set<.*>|std::unordered_set<.*>|vector<.*>|std::vector<.*>)$ + - has: + # nthChild: 2 + stopBy: end + pattern: $VAR + - not: + inside: + stopBy: end + has: + kind: storage_class_specifier diff --git a/rules/cpp/security/std-vector-invalidation-cpp.yml b/rules/cpp/security/std-vector-invalidation-cpp.yml new file mode 100644 index 00000000..39fc091f --- /dev/null +++ b/rules/cpp/security/std-vector-invalidation-cpp.yml @@ -0,0 +1,150 @@ +id: std-vector-invalidation-cpp +language: cpp +severity: warning +message: >- + Modifying an `std::vector` while iterating over it could cause the + container to reallocate, triggering memory corruption. +note: >- + [CWE-416: Use After Free. + [REFERENCES] + - https://wiki.sei.cmu.edu/confluence/display/c/MEM30-C.+Do+not+access+freed+memory + - https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP54-CPP.+Do+not+access+an+object+outside+of+its+lifetime + +ast-grep-essentials: true + +rule: + kind: call_expression + all: + - any: + - pattern: $CONTAINER.erase($IT) + all: + - all: + - not: + follows: + stopBy: end + pattern: $CONTAINER.erase($IT) + - not: + precedes: + stopBy: end + pattern: $CONTAINER.erase($IT) + - not: + inside: + stopBy: end + kind: assignment_expression + has: + kind: identifier + pattern: $IT + nthChild: 1 + - pattern: $CONTAINER.assign($$$) + - pattern: $CONTAINER.clear($$$) + - pattern: $CONTAINER.emplace_back($$$) + - pattern: $CONTAINER.insert($$$) + - pattern: $CONTAINER.resize($$$) + - pattern: $CONTAINER.push_back($$$) + - pattern: $CONTAINER.reserve($$$) + - pattern: $CONTAINER.shrink_to_fit($$$) + - pattern: $CONTAINER.resize($$$) + - pattern: $CONTAINER.pop_back($$$) + - not: + inside: + stopBy: end + kind: for_statement + has: + stopBy: end + any: + - kind: break_statement + - kind: continue_statement + - kind: return_statement + - kind: goto_statement + - inside: + stopBy: end + kind: for_statement + any: + - all: + - has: + kind: declaration + any: + - pattern: std::vector<$TY>::$IT_TYPE $IT = $CONTAINER.begin() + - all: + - has: + kind: dependent_type + has: + stopBy: end + pattern: std::vector<$TY>::$IT_TYPE + - has: + stopBy: end + kind: init_declarator + all: + - has: + pattern: $IT + - has: + pattern: $CONTAINER.begin() + - has: + kind: binary_expression + any: + - pattern: $IT != $CONTAINER.end() + - has: + kind: update_expression + any: + - pattern: ++$IT + - pattern: $IT++ + - all: + - has: + kind: declaration + any: + - pattern: std::vector<$TY>::$IT_TYPE $IT = $CONTAINER.rbegin() + - has: + stopBy: end + pattern: std::vector<$TY>::$IT_TYPE $IT = $CONTAINER.rbegin() + - all: + - has: + kind: dependent_type + has: + stopBy: end + pattern: std::vector<$TY>::$IT_TYPE + - has: + stopBy: end + kind: init_declarator + all: + - has: + pattern: $IT + - has: + pattern: $CONTAINER.rbegin() + - has: + kind: binary_expression + any: + - pattern: $IT != $CONTAINER.rend() + - has: + kind: update_expression + any: + - pattern: ++$IT + - pattern: $IT++ + - all: + - has: + kind: declaration + any: + - pattern: std::vector<$TY>::$IT_TYPE $IT = $CONTAINER.begin(), $IT_END = $CONTAINER.end() + - pattern: std::vector<$TY>::$IT_TYPE $IT = $CONTAINER.rbegin(), $IT_END = $CONTAINER.rend() + - has: + stopBy: end + any: + - pattern: std::vector<$TY>::$IT_TYPE $IT = $CONTAINER.begin(), $IT_END = $CONTAINER.end() + - pattern: std::vector<$TY>::$IT_TYPE $IT = $CONTAINER.rbegin(), $IT_END = $CONTAINER.rend() + - has: + kind: binary_expression + any: + - pattern: $IT != $IT_END + - has: + kind: update_expression + any: + - pattern: ++$IT + - pattern: $IT++ + - all: + - not: + has: + stopBy: end + kind: ERROR + - not: + inside: + stopBy: end + kind: ERROR diff --git a/tests/__snapshots__/std-return-data-cpp-snapshot.yml b/tests/__snapshots__/std-return-data-cpp-snapshot.yml new file mode 100644 index 00000000..e6f84d13 --- /dev/null +++ b/tests/__snapshots__/std-return-data-cpp-snapshot.yml @@ -0,0 +1,48 @@ +id: std-return-data-cpp +snapshots: + ? | + int *return_vector_data() { + std::vector v; + return v.data(); + } + : labels: + - source: return v.data(); + style: primary + start: 50 + end: 66 + - source: int + style: secondary + start: 0 + end: 3 + - source: '*return_vector_data()' + style: secondary + start: 4 + end: 25 + - source: |- + int *return_vector_data() { + std::vector v; + return v.data(); + } + style: secondary + start: 0 + end: 68 + - source: std::vector + style: secondary + start: 29 + end: 45 + - source: v + style: secondary + start: 46 + end: 47 + - source: |- + { + std::vector v; + return v.data(); + } + style: secondary + start: 26 + end: 68 + - source: std::vector v; + style: secondary + start: 29 + end: 48 diff --git a/tests/__snapshots__/std-vector-invalidation-cpp-snapshot.yml b/tests/__snapshots__/std-vector-invalidation-cpp-snapshot.yml new file mode 100644 index 00000000..225e93ea --- /dev/null +++ b/tests/__snapshots__/std-vector-invalidation-cpp-snapshot.yml @@ -0,0 +1,30 @@ +id: std-vector-invalidation-cpp +snapshots: + ? "void loop_variant_5(std::vector &vec) {\n for(std::vector::iterator it = vec.begin(); it != vec.end(); ++it) {\n if (should_erase(*it)) {\n // ruleid: std-vector-invalidation\n vec.erase(it);\n }\n }\n}\nvoid loop_variant_6(std::vector &vec) {\n for(std::vector::iterator it = vec.begin(); it != vec.end(); it++) {\n if (should_erase(*it)) {\n // ruleid: std-vector-invalidation\n vec.erase(it);\n }\n }\n}\nvoid loop_variant_7(std::vector &vec) {\n for(std::vector::iterator it = vec.rbegin(); it != vec.rend(); ++it) {\n if (should_erase(*it)) {\n // ruleid: std-vector-invalidation\n vec.erase(it);\n }\n }\n}\nvoid loop_variant_8(std::vector &vec) {\n for(std::vector::iterator it = vec.rbegin(); it != vec.rend(); it++) {\n if (should_erase(*it)) {\n // ruleid: std-vector-invalidation\n vec.erase(it);\n }\n }\n}\nvoid loop_variant_9(std::vector &vec) {\n for(std::vector::iterator it = vec.begin(), end = vec.end(); it != end; ++it) {\n if (should_erase(*it)) {\n // ruleid: std-vector-invalidation\n vec.erase(it);\n }\n }\n}\nvoid loop_variant_10(std::vector &vec) {\n for(std::vector::iterator it = vec.begin(), end = vec.end(); it != end; it++) {\n if (should_erase(*it)) {\n // ruleid: std-vector-invalidation\n vec.erase(it);\n }\n }\n}\nvoid loop_variant_11(std::vector &vec) {\n for(std::vector::iterator it = vec.rbegin(), end = vec.rend(); it != end; ++it) {\n if (should_erase(*it)) {\n // ruleid: std-vector-invalidation\n vec.erase(it);\n }\n }\n}\nvoid loop_variant_12(std::vector &vec) {\n for(std::vector::iterator it = vec.rbegin(), end = vec.rend(); it != end; it++) {\n if (should_erase(*it)) {\n // ruleid: std-vector-invalidation\n vec.erase(it);\n }\n }\n} \nvoid f(std::vector &vec, std::vector &other_vec) {\n for(std::vector::iterator it = vec.begin(); it != vec.end(); it++) {\n if (foo()) {\n // ruleid: std-vector-invalidation\n vec.push_back(0);\n\n // Modifying a different container is OK\n // ok: std-vector-invalidation\n other_vec.push_back(0);\n }\n }\n}\n" + : labels: + - source: vec.erase(it) + style: primary + start: 197 + end: 210 + - source: std::vector::iterator it = vec.begin(); + style: secondary + start: 51 + end: 95 + - source: it != vec.end() + style: secondary + start: 96 + end: 111 + - source: ++it + style: secondary + start: 113 + end: 117 + - source: |- + for(std::vector::iterator it = vec.begin(); it != vec.end(); ++it) { + if (should_erase(*it)) { + // ruleid: std-vector-invalidation + vec.erase(it); + } + } + style: secondary + start: 47 + end: 221 diff --git a/tests/cpp/std-return-data-cpp-test.yml b/tests/cpp/std-return-data-cpp-test.yml new file mode 100644 index 00000000..cc161c40 --- /dev/null +++ b/tests/cpp/std-return-data-cpp-test.yml @@ -0,0 +1,15 @@ +id: std-return-data-cpp +valid: + - | + class Wrapper { + std::vector v; + int *return_vector_begin_iterator() { + return v.data(); + } + } +invalid: + - | + int *return_vector_data() { + std::vector v; + return v.data(); + } diff --git a/tests/cpp/std-vector-invalidation-cpp-test.yml b/tests/cpp/std-vector-invalidation-cpp-test.yml new file mode 100644 index 00000000..0e05a504 --- /dev/null +++ b/tests/cpp/std-vector-invalidation-cpp-test.yml @@ -0,0 +1,103 @@ +id: std-vector-invalidation-cpp +valid: + - | + void f(std::vector &vec) { + for (std::vector::iterator it = vec.begin(); it != vec.end(); ++it) { + if (should_erase(*it)) { + // This is the correct way to iterate while erasing + // ok: std-vector-invalidation + it = vec.erase(it); + } else { + ++it; + } + } + } + bool isInList(const TCHAR *token2Find, std::vector ¶ms, bool eraseArg = true){ + for (std::vector::iterator = params.begin(); it != params.end(); ++it) + { + if (lstrcmp(token2Find, it->c_str()) == 0){ + // ok: std-vector-invalidation + if (eraseArg) params.erase(it); + return true; + } + } + return false; + } +invalid: + - | + void loop_variant_5(std::vector &vec) { + for(std::vector::iterator it = vec.begin(); it != vec.end(); ++it) { + if (should_erase(*it)) { + // ruleid: std-vector-invalidation + vec.erase(it); + } + } + } + void loop_variant_6(std::vector &vec) { + for(std::vector::iterator it = vec.begin(); it != vec.end(); it++) { + if (should_erase(*it)) { + // ruleid: std-vector-invalidation + vec.erase(it); + } + } + } + void loop_variant_7(std::vector &vec) { + for(std::vector::iterator it = vec.rbegin(); it != vec.rend(); ++it) { + if (should_erase(*it)) { + // ruleid: std-vector-invalidation + vec.erase(it); + } + } + } + void loop_variant_8(std::vector &vec) { + for(std::vector::iterator it = vec.rbegin(); it != vec.rend(); it++) { + if (should_erase(*it)) { + // ruleid: std-vector-invalidation + vec.erase(it); + } + } + } + void loop_variant_9(std::vector &vec) { + for(std::vector::iterator it = vec.begin(), end = vec.end(); it != end; ++it) { + if (should_erase(*it)) { + // ruleid: std-vector-invalidation + vec.erase(it); + } + } + } + void loop_variant_10(std::vector &vec) { + for(std::vector::iterator it = vec.begin(), end = vec.end(); it != end; it++) { + if (should_erase(*it)) { + // ruleid: std-vector-invalidation + vec.erase(it); + } + } + } + void loop_variant_11(std::vector &vec) { + for(std::vector::iterator it = vec.rbegin(), end = vec.rend(); it != end; ++it) { + if (should_erase(*it)) { + // ruleid: std-vector-invalidation + vec.erase(it); + } + } + } + void loop_variant_12(std::vector &vec) { + for(std::vector::iterator it = vec.rbegin(), end = vec.rend(); it != end; it++) { + if (should_erase(*it)) { + // ruleid: std-vector-invalidation + vec.erase(it); + } + } + } + void f(std::vector &vec, std::vector &other_vec) { + for(std::vector::iterator it = vec.begin(); it != vec.end(); it++) { + if (foo()) { + // ruleid: std-vector-invalidation + vec.push_back(0); + + // Modifying a different container is OK + // ok: std-vector-invalidation + other_vec.push_back(0); + } + } + }