CMFSUPPORT-3863. COVERITY TEST. DO NOT MERGE#49
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if ((file = fopen(fpath, "w"))) | ||
| { | ||
| fprintf(file,"%s",str); | ||
| fprintf(file,"%s%s",str); |
Check warning
Code scanning / CodeQL
Too few arguments to formatting function Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
In general, to fix “too few arguments to formatting function” issues, ensure that the number and types of arguments following the format string match the conversion specifiers in the format. You can either adjust the format string to match the existing arguments or add the missing arguments to match the format specifiers.
Here, advsec_write_to_file is meant to write a string str into the file fpath. The call fprintf(file,"%s%s",str); indicates that only str is expected to be written; there is no second string argument, and nothing else in the function suggests a need to print two strings. The minimal and behavior-preserving fix is therefore to correct the format string to use a single %s, i.e., fprintf(file, "%s", str);. This keeps the existing functionality (write exactly str to the file) while removing the undefined behavior.
Concretely:
- In
source/AdvSecurityDml/cosa_adv_security_internal.c, withinadvsec_write_to_file, change line 294 fromfprintf(file,"%s%s",str);tofprintf(file,"%s",str);. - No new methods, imports, or definitions are needed; we rely on the standard
fprintfalready in use.
| @@ -291,7 +291,7 @@ | ||
|
|
||
| if ((file = fopen(fpath, "w"))) | ||
| { | ||
| fprintf(file,"%s%s",str); | ||
| fprintf(file,"%s",str); | ||
| fclose(file); | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
Pull request overview
This PR introduces a deliberate format string bug and suppresses compiler warnings to prevent its detection. The title and description explicitly state this is a "COVERITY TEST" and "DO NOT MERGE", indicating this is likely a test to verify that static analysis tools (like Coverity) can detect format string vulnerabilities even when compiler warnings are disabled.
Changes:
- Introduced a format string bug in fprintf with mismatched format specifiers and arguments
- Added compiler flags to suppress format string warnings that would normally catch this bug
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| source/AdvSecurityDml/cosa_adv_security_internal.c | Modified fprintf call to have incorrect format string (two %s specifiers with only one argument) |
| source/AdvSecurityDml/Makefile.am | Added -Wno-format flags and removed -Werror to suppress compiler warnings that would detect the format string bug |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| AM_LDFLAGS = -lccsp_common -lsysevent -lwebconfig_framework -lmsgpackc -ltrower-base64 | ||
|
|
||
| AM_CPPFLAGS = -Wall -Werror | ||
| AM_CPPFLAGS = -Wall -Wno-format |
There was a problem hiding this comment.
Replacing -Werror with -Wno-format removes the "warnings as errors" enforcement and suppresses format string warnings. This is problematic for two reasons: (1) it allows format string bugs to go undetected, including the critical bug on line 294 of cosa_adv_security_internal.c, and (2) it removes the previous policy of treating all warnings as errors, which weakens code quality enforcement.
| if ((file = fopen(fpath, "w"))) | ||
| { | ||
| fprintf(file,"%s",str); | ||
| fprintf(file,"%s%s",str); |
There was a problem hiding this comment.
The fprintf format string has two format specifiers ("%s%s") but only one argument (str) is provided. This will cause undefined behavior as fprintf will attempt to read a second argument from the stack that doesn't exist. The format string should be "%s" with one argument, not "%s%s".
| fprintf(file,"%s%s",str); | |
| fprintf(file, "%s", str); |
| ########################################################################## | ||
| AM_CFLAGS = -D_ANSC_LINUX | ||
| AM_CFLAGS += -D_ANSC_USER | ||
| AM_CFLAGS += -Wno-format |
There was a problem hiding this comment.
Adding -Wno-format suppresses compiler warnings for format string mismatches, which would hide serious bugs like format string vulnerabilities and incorrect argument counts. This flag is masking the bug in line 294 of cosa_adv_security_internal.c where fprintf has mismatched format specifiers and arguments. Format warnings should not be suppressed as they catch critical security and correctness issues.
| if ((file = fopen(fpath, "w"))) | ||
| { | ||
| fprintf(file,"%s",str); | ||
| fprintf(file,"%s%s",str); |
There was a problem hiding this comment.
Coverity Issue - Printf arg count mismatch
the format string requires additional arguments
Medium Impact, CWE-685
PW.TOO_FEW_PRINTF_ARGS
| if ((file = fopen(fpath, "w"))) | ||
| { | ||
| fprintf(file,"%s",str); | ||
| fprintf(file,"%s%s",str); |
There was a problem hiding this comment.
Coverity Issue - Missing argument to printf format specifier
No argument for format specifier "%s".
Medium Impact, CWE-685
PRINTF_ARGS
|
Close test PR |
2 participants
DO NOT MERGE