diff --git a/rules/S7128/cfamily/metadata.json b/rules/S7128/cfamily/metadata.json index 21c4c69edd..f87a46740c 100644 --- a/rules/S7128/cfamily/metadata.json +++ b/rules/S7128/cfamily/metadata.json @@ -1,5 +1,5 @@ { - "title": "\"strlen\" should not be called on incremented pointer", + "title": "\"strlen\" should not be called on incremented pointers", "type": "CODE_SMELL", "status": "ready", "remediation": { @@ -8,7 +8,7 @@ }, "tags": [ ], - "defaultSeverity": "Minor", + "defaultSeverity": "Major", "ruleSpecification": "RSPEC-7128", "sqKey": "S7128", "scope": "All", diff --git a/rules/S7128/cfamily/rule.adoc b/rules/S7128/cfamily/rule.adoc index 677047c66f..9544a6e091 100644 --- a/rules/S7128/cfamily/rule.adoc +++ b/rules/S7128/cfamily/rule.adoc @@ -1,26 +1,26 @@ -This rule raises the issue of calls `strlen(ptr + 1)`, for which the intent is unclear. +Calling `strlen(ptr + 1)` looks very similar to the more common pattern `strlen(ptr) + 1` and has unclear intent. == Why is this an issue? The call `strlen(ptr) + 1` is commonly used to compute the memory required to copy a string: -the one is needed to allocate space for a null-terminator. +the `+ 1` is needed to allocate space for a null-terminator. -The very similar call `strlen(ptr + 1)` that differs by placement of the right parenthesis, +The very similar call `strlen(ptr + 1)` that differs only by the placement of the right parenthesis, produces vastly different results: - * non-empty string computes the length of string without the first character (reduced by one) - * if the string is empty, traverse memory after it until the null terminator (`'\0'`) is found. + * non-empty string computes the length of string without the first character (`strlen(ptr) - 1`) + * if the string is empty, it traverses neighboring memory until a null terminator (`'\0'`) is found. The latter may lead to undefined behavior. -Due above, the `strlen(ptr + 1)` calls are likely to be unintended -and result from the type. This rule raises issues on such calls. +In consequence, calls to `strlen(ptr + 1)` are likely to be unintended +and the result of a typo. This rule raises issues on such calls. === Code examples ==== Noncompliant code example -[source,c] +[source,c,diff-id=1,diff-type=noncompliant] ---- size_t size = strlen(ptr + 1); // Noncompliant ---- @@ -29,7 +29,7 @@ size_t size = strlen(ptr + 1); // Noncompliant If the use of `strlen(ptr + 1)` was an typo: -[source,c] +[source,c,diff-id=1,diff-type=compliant] ---- size_t size = strlen(ptr) + 1; // Compliant ---- @@ -52,13 +52,13 @@ For example, it can lead to buffer overflow if the operation was used to compute [source,c] ---- char* duplicate(char const* source) { - char result = (char*)malloc(strlen(source + 1)); // Should be malloc(strlen(source) + 1)) + char* result = (char*)malloc(strlen(source + 1)); // Should be malloc(strlen(source) + 1)) strcpy(result, source); // Writes two characters outside of "result" buffer return result; } ---- -Buffer overflow, as other undefined behavior, has a wide range of effects. +Buffer overflows, as other undefined behavior, have a wide range of effects. In many cases, the access works by accident and succeeds at writing or reading a value. However, it can start misbehaving at any time. If compilation flags, compiler, platform, or runtime environment change,