Skip to content

[clang][analyzer] Support fprintf in the SecuritySyntaxChecker #73247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 24, 2023
Merged

[clang][analyzer] Support fprintf in the SecuritySyntaxChecker #73247

merged 1 commit into from
Nov 24, 2023

Conversation

benshi001
Copy link
Member

No description provided.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Nov 23, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 23, 2023

@llvm/pr-subscribers-clang

Author: Ben Shi (benshi001)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/73247.diff

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp (+7-6)
  • (modified) clang/test/Analysis/security-syntax-checks.c (+12-3)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index 1a88615f01697cb..e96f8f131e0034b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -162,7 +162,7 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
     .Cases("sscanf", "swscanf", "vsscanf", "vswscanf", "swprintf",
            "snprintf", "vswprintf", "vsnprintf", "memcpy", "memmove",
            &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
-    .Cases("strncpy", "strncat", "memset",
+    .Cases("strncpy", "strncat", "memset", "fprintf",
            &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
     .Case("drand48", &WalkAST::checkCall_rand)
     .Case("erand48", &WalkAST::checkCall_rand)
@@ -737,10 +737,10 @@ void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) {
 // Check: Any use of 'sprintf', 'vsprintf', 'scanf', 'wscanf', 'fscanf',
 //        'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf',
 //        'swscanf', 'vsscanf', 'vswscanf', 'swprintf', 'snprintf', 'vswprintf',
-//        'vsnprintf', 'memcpy', 'memmove', 'strncpy', 'strncat', 'memset'
-//        is deprecated since C11.
+//        'vsnprintf', 'memcpy', 'memmove', 'strncpy', 'strncat', 'memset',
+//        'fprintf' is deprecated since C11.
 //
-//        Use of 'sprintf', 'vsprintf', 'scanf', 'wscanf','fscanf',
+//        Use of 'sprintf', 'fprintf', 'vsprintf', 'scanf', 'wscanf', 'fscanf',
 //        'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf',
 //        'swscanf', 'vsscanf', 'vswscanf' without buffer limitations
 //        is insecure.
@@ -768,8 +768,9 @@ void WalkAST::checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE,
   int ArgIndex =
       llvm::StringSwitch<int>(Name)
           .Cases("scanf", "wscanf", "vscanf", "vwscanf", 0)
-          .Cases("sprintf", "vsprintf", "fscanf", "fwscanf", "vfscanf",
-                 "vfwscanf", "sscanf", "swscanf", "vsscanf", "vswscanf", 1)
+          .Cases("fscanf", "fwscanf", "vfscanf", "vfwscanf", "sscanf",
+                 "swscanf", "vsscanf", "vswscanf", 1)
+          .Cases("sprintf", "vsprintf", "fprintf", 1)
           .Cases("swprintf", "snprintf", "vswprintf", "vsnprintf", "memcpy",
                  "memmove", "memset", "strncpy", "strncat", DEPR_ONLY)
           .Default(UNKNOWN_CALL);
diff --git a/clang/test/Analysis/security-syntax-checks.c b/clang/test/Analysis/security-syntax-checks.c
index 392a65ff5f167ce..2c904f41623fcd1 100644
--- a/clang/test/Analysis/security-syntax-checks.c
+++ b/clang/test/Analysis/security-syntax-checks.c
@@ -5,15 +5,24 @@
 // RUN: %clang_analyze_cc1 %s -verify -std=gnu99 \
 // RUN:   -analyzer-checker=security.insecureAPI
 
+#include "Inputs/system-header-simulator.h"
+
+extern FILE *fp;
+extern char buf[128];
+
 void builtin_function_call_crash_fixes(char *c) {
   __builtin_strncpy(c, "", 6);
   __builtin_memset(c, '\0', (0));
   __builtin_memcpy(c, c, 0);
+  __builtin_sprintf(buf, "%s", c);
+  __builtin_fprintf(fp, "%s", c);
 
 #if __STDC_VERSION__ > 199901
-  // expected-warning@-5{{Call to function 'strncpy' is insecure as it does not provide security checks introduced in the C11 standard.}}
-  // expected-warning@-5{{Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard.}}
-  // expected-warning@-5{{Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'strncpy' is insecure as it does not provide security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'sprintf' is insecure as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'fprintf' is insecure as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard.}}
 #else
   // expected-no-diagnostics
 #endif

@llvmbot
Copy link
Member

llvmbot commented Nov 23, 2023

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Ben Shi (benshi001)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/73247.diff

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp (+7-6)
  • (modified) clang/test/Analysis/security-syntax-checks.c (+12-3)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index 1a88615f01697cb..e96f8f131e0034b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -162,7 +162,7 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
     .Cases("sscanf", "swscanf", "vsscanf", "vswscanf", "swprintf",
            "snprintf", "vswprintf", "vsnprintf", "memcpy", "memmove",
            &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
-    .Cases("strncpy", "strncat", "memset",
+    .Cases("strncpy", "strncat", "memset", "fprintf",
            &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
     .Case("drand48", &WalkAST::checkCall_rand)
     .Case("erand48", &WalkAST::checkCall_rand)
@@ -737,10 +737,10 @@ void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) {
 // Check: Any use of 'sprintf', 'vsprintf', 'scanf', 'wscanf', 'fscanf',
 //        'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf',
 //        'swscanf', 'vsscanf', 'vswscanf', 'swprintf', 'snprintf', 'vswprintf',
-//        'vsnprintf', 'memcpy', 'memmove', 'strncpy', 'strncat', 'memset'
-//        is deprecated since C11.
+//        'vsnprintf', 'memcpy', 'memmove', 'strncpy', 'strncat', 'memset',
+//        'fprintf' is deprecated since C11.
 //
-//        Use of 'sprintf', 'vsprintf', 'scanf', 'wscanf','fscanf',
+//        Use of 'sprintf', 'fprintf', 'vsprintf', 'scanf', 'wscanf', 'fscanf',
 //        'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf',
 //        'swscanf', 'vsscanf', 'vswscanf' without buffer limitations
 //        is insecure.
@@ -768,8 +768,9 @@ void WalkAST::checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE,
   int ArgIndex =
       llvm::StringSwitch<int>(Name)
           .Cases("scanf", "wscanf", "vscanf", "vwscanf", 0)
-          .Cases("sprintf", "vsprintf", "fscanf", "fwscanf", "vfscanf",
-                 "vfwscanf", "sscanf", "swscanf", "vsscanf", "vswscanf", 1)
+          .Cases("fscanf", "fwscanf", "vfscanf", "vfwscanf", "sscanf",
+                 "swscanf", "vsscanf", "vswscanf", 1)
+          .Cases("sprintf", "vsprintf", "fprintf", 1)
           .Cases("swprintf", "snprintf", "vswprintf", "vsnprintf", "memcpy",
                  "memmove", "memset", "strncpy", "strncat", DEPR_ONLY)
           .Default(UNKNOWN_CALL);
diff --git a/clang/test/Analysis/security-syntax-checks.c b/clang/test/Analysis/security-syntax-checks.c
index 392a65ff5f167ce..2c904f41623fcd1 100644
--- a/clang/test/Analysis/security-syntax-checks.c
+++ b/clang/test/Analysis/security-syntax-checks.c
@@ -5,15 +5,24 @@
 // RUN: %clang_analyze_cc1 %s -verify -std=gnu99 \
 // RUN:   -analyzer-checker=security.insecureAPI
 
+#include "Inputs/system-header-simulator.h"
+
+extern FILE *fp;
+extern char buf[128];
+
 void builtin_function_call_crash_fixes(char *c) {
   __builtin_strncpy(c, "", 6);
   __builtin_memset(c, '\0', (0));
   __builtin_memcpy(c, c, 0);
+  __builtin_sprintf(buf, "%s", c);
+  __builtin_fprintf(fp, "%s", c);
 
 #if __STDC_VERSION__ > 199901
-  // expected-warning@-5{{Call to function 'strncpy' is insecure as it does not provide security checks introduced in the C11 standard.}}
-  // expected-warning@-5{{Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard.}}
-  // expected-warning@-5{{Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'strncpy' is insecure as it does not provide security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'sprintf' is insecure as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard.}}
+  // expected-warning@-7{{Call to function 'fprintf' is insecure as it does not provide bounding of the memory buffer or security checks introduced in the C11 standard.}}
 #else
   // expected-no-diagnostics
 #endif

Copy link

github-actions bot commented Nov 23, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@Xazax-hun
Copy link
Collaborator

This is unrelated to this particular patch, but it would be great if this checker would also give suggestions what other functions should be used instead.

@benshi001 benshi001 merged commit dd0b3c2 into llvm:main Nov 24, 2023
@benshi001 benshi001 deleted the csa-fprintf branch November 24, 2023 08:24
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Nov 30, 2023
Local branch amd-gfx dffa6f9 Merged main:c0722478d5c1 into amd-gfx:94e1716d2b22
Remote branch main dd0b3c2 [clang][analyzer] Support `fprintf` in the SecuritySyntaxChecker (llvm#73247)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants