[DO NOT MERGE] Dummy PR to check for coverity integration#50
[DO NOT MERGE] Dummy PR to check for coverity integration#50sowmiyachelliah wants to merge 1 commit intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR appears to be a dummy change set intended to validate Coverity/static-analysis integration by intentionally introducing patterns that analyzers should flag.
Changes:
- Introduces an intentional file-handle leak on an error path in
advsec_read_from_file. - Adds a use-after-free/NULL
%sformatting issue inCosaAdvSecFetchSbConfig’s JSON parse failure path. - Adds an unused local variable in
CosaAdvSecGetLookupTimeoutExceededCount.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FILE *fp; | ||
| char buf[COMMAND_MAX] = {0}; | ||
| // LOW IMPACT ISSUE: Unused variable declared but never used | ||
| int retry_attempts = 0; |
There was a problem hiding this comment.
retry_attempts is declared but never used, which will trigger an unused-variable warning (and can fail builds if warnings are treated as errors). Remove it or use it in the intended retry logic.
| int retry_attempts = 0; |
| // MEDIUM IMPACT ISSUE: Resource leak - file not closed in error path | ||
| //fclose(file); |
There was a problem hiding this comment.
The snprintf error path returns without closing file, which leaks the file descriptor/FILE* when fopen succeeded. Close the file before returning (or refactor to a single exit path that always fcloses when file is non-NULL).
| // MEDIUM IMPACT ISSUE: Resource leak - file not closed in error path | |
| //fclose(file); | |
| fclose(file); |
| json = cJSON_Parse(data); | ||
| if (data != NULL) | ||
| { | ||
| free(data); | ||
| data = NULL; | ||
| } | ||
| if( !json ) | ||
| { | ||
| CcspTraceWarning(("json file parser error %s:%d\n", __FUNCTION__,__LINE__)); | ||
| // HIGH IMPACT ISSUE: Use after free - data is freed above but used here | ||
| CcspTraceError(("Failed to parse JSON data: %s\n", data)); | ||
| return ANSC_STATUS_FAILURE; |
There was a problem hiding this comment.
data is freed (and set to NULL) before this error log, but the log still formats it with %s. Passing a freed/NULL pointer to %s is undefined behavior and can crash on JSON parse failure. Log before freeing, or preserve the buffer until after all uses (or log the file path / length instead).
| fclose(file); | ||
| // MEDIUM IMPACT ISSUE: Resource leak - file not closed in error path | ||
| //fclose(file); | ||
| return 0; |
There was a problem hiding this comment.
Coverity Issue - Resource leak
Variable "file" going out of scope leaks the storage it points to.
High Impact, CWE-404
RESOURCE_LEAK
[DO NOT MERGE] Dummy PR to check for coverity integration