[clang] [clang][analyzer] Mark `_lstrcatA`, `_strcpyA`, and `_strdup` as insecure on Windows in `SecuritySyntaxChecker` (PR #164183)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 19 14:45:05 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Stefan Milenkovic (stemil01)
<details>
<summary>Changes</summary>
Addresses issue #<!-- -->103038.
Some of the functions mentioned in that issue are already covered or do not exist under the specified names. Therefore, this patch adds only the three functions listed above.
There is a separate commit for `clang-format` style changes since they seem to be inconsistent with the rest of `Checkers.td` file.
---
Full diff: https://github.com/llvm/llvm-project/pull/164183.diff
2 Files Affected:
- (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+15)
- (modified) clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp (+215-2)
``````````diff
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index ffae3b9310979..547a6c9360fee 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -869,6 +869,16 @@ def getpw : Checker<"getpw">,
Dependencies<[SecuritySyntaxChecker]>,
Documentation<HasDocumentation>;
+def lstrcatA : Checker<"lstrcatA">,
+ HelpText<"Warn on uses of the 'lstrcatA' function">,
+ Dependencies<[SecuritySyntaxChecker]>,
+ Documentation<HasDocumentation>;
+
+def lstrcpyA : Checker<"lstrcpyA">,
+ HelpText<"Warn on uses of the 'lstrcpyA' function">,
+ Dependencies<[SecuritySyntaxChecker]>,
+ Documentation<HasDocumentation>;
+
def mktemp : Checker<"mktemp">,
HelpText<"Warn on uses of the 'mktemp' function">,
Dependencies<[SecuritySyntaxChecker]>,
@@ -890,6 +900,11 @@ def strcpy : Checker<"strcpy">,
Dependencies<[SecuritySyntaxChecker]>,
Documentation<HasDocumentation>;
+def strdup : Checker<"strdup">,
+ HelpText<"Warn on uses of the '_strdup' function">,
+ Dependencies<[SecuritySyntaxChecker]>,
+ Documentation<HasDocumentation>;
+
def vfork : Checker<"vfork">,
HelpText<"Warn on uses of the 'vfork' function">,
Dependencies<[SecuritySyntaxChecker]>,
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index 5e75c1c4a3abd..c97f4fb18e1c6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -10,14 +10,19 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
#include "clang/AST/StmtVisitor.h"
+#include "clang/AST/TypeBase.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
-#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/raw_ostream.h"
@@ -40,9 +45,12 @@ struct ChecksFilter {
bool check_bzero = false;
bool check_gets = false;
bool check_getpw = false;
+ bool check_lstrcatA = false;
+ bool check_lstrcpyA = false;
bool check_mktemp = false;
bool check_mkstemp = false;
bool check_strcpy = false;
+ bool check_strdup = false;
bool check_DeprecatedOrUnsafeBufferHandling = false;
bool check_rand = false;
bool check_vfork = false;
@@ -55,9 +63,12 @@ struct ChecksFilter {
CheckerNameRef checkName_bzero;
CheckerNameRef checkName_gets;
CheckerNameRef checkName_getpw;
+ CheckerNameRef checkName_lstrcatA;
+ CheckerNameRef checkName_lstrcpyA;
CheckerNameRef checkName_mktemp;
CheckerNameRef checkName_mkstemp;
CheckerNameRef checkName_strcpy;
+ CheckerNameRef checkName_strdup;
CheckerNameRef checkName_DeprecatedOrUnsafeBufferHandling;
CheckerNameRef checkName_rand;
CheckerNameRef checkName_vfork;
@@ -104,9 +115,12 @@ class WalkAST : public StmtVisitor<WalkAST> {
void checkCall_bzero(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_gets(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_lstrcatA(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_lstrcpyA(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_mkstemp(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_strdup(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD);
void checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE,
const FunctionDecl *FD);
@@ -150,12 +164,15 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
.Case("bzero", &WalkAST::checkCall_bzero)
.Case("gets", &WalkAST::checkCall_gets)
.Case("getpw", &WalkAST::checkCall_getpw)
+ .Case("lstrcatA", &WalkAST::checkCall_lstrcatA)
+ .Case("lstrcpyA", &WalkAST::checkCall_lstrcpyA)
.Case("mktemp", &WalkAST::checkCall_mktemp)
.Case("mkstemp", &WalkAST::checkCall_mkstemp)
.Case("mkdtemp", &WalkAST::checkCall_mkstemp)
.Case("mkstemps", &WalkAST::checkCall_mkstemp)
.Cases({"strcpy", "__strcpy_chk"}, &WalkAST::checkCall_strcpy)
.Cases({"strcat", "__strcat_chk"}, &WalkAST::checkCall_strcat)
+ .Case("_strdup", &WalkAST::checkCall_strdup)
.Cases({"sprintf", "vsprintf", "scanf", "wscanf", "fscanf", "fwscanf",
"vscanf", "vwscanf", "vfscanf", "vfwscanf"},
&WalkAST::checkDeprecatedOrUnsafeBufferHandling)
@@ -542,6 +559,145 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
CELoc, CE->getCallee()->getSourceRange());
}
+//===----------------------------------------------------------------------===//
+// Check: Any use of 'lstrcatA' is insecure.
+//
+// CWE-119: Improper Restriction of Operations within
+// the Bounds of a Memory Buffer
+//===----------------------------------------------------------------------===//
+
+void WalkAST::checkCall_lstrcatA(const CallExpr *CE, const FunctionDecl *FD) {
+ if (!filter.check_lstrcatA)
+ return;
+
+ // Verify that the operating system is Windows
+ if (!FD->getASTContext().getTargetInfo().getTriple().isOSWindows())
+ return;
+
+ // Checking the type of the arguments
+ const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>();
+ if (!FPT)
+ return;
+
+ // Verify the function takes two arguments, three in the _chk version.
+ int numArgs = FPT->getNumParams();
+ if (numArgs != 2)
+ return;
+
+ // Verify the type for both arguments.
+ for (int i = 0; i < 2; i++) {
+ // Verify that the arguments are pointers.
+ const PointerType *PT = FPT->getParamType(i)->getAs<PointerType>();
+ if (!PT)
+ return;
+
+ // Verify that the argument is a 'char*'.
+ if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().CharTy)
+ return;
+ }
+ // Second argument has to have 'const' qualifier
+ if (!FPT->getParamType(1)
+ ->getAs<PointerType>()
+ ->getPointeeType()
+ .isConstQualified())
+ return;
+
+ // Verify that the function returns 'char *'
+ const PointerType *PTReturnType = FPT->getReturnType()->getAs<PointerType>();
+ if (!PTReturnType)
+ return;
+ if (PTReturnType->getPointeeType().getUnqualifiedType() !=
+ BR.getContext().CharTy)
+ return;
+
+ // Issue a warning.
+ PathDiagnosticLocation CELoc =
+ PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+ BR.EmitBasicReport(AC->getDecl(), filter.checkName_strcpy,
+ "Potential insecure memory buffer bounds restriction in "
+ "call 'lstrcatA'",
+ "Security",
+ "Call to function 'lstrcatA' is insecure as it does not "
+ "provide bounding of the memory buffer. Consider "
+ "using 'StringCchCat' instead. CWE-119.",
+ CELoc, CE->getCallee()->getSourceRange());
+}
+
+//===----------------------------------------------------------------------===//
+// Check: Any use of 'lstrcpyA' is insecure.
+//
+// CWE-119: Improper Restriction of Operations within
+// the Bounds of a Memory Buffer
+//===----------------------------------------------------------------------===//
+
+void WalkAST::checkCall_lstrcpyA(const CallExpr *CE, const FunctionDecl *FD) {
+ if (!filter.check_lstrcpyA)
+ return;
+
+ // Verify that the operating system is Windows
+ if (!FD->getASTContext().getTargetInfo().getTriple().isOSWindows())
+ return;
+
+ // Checking the type of the arguments
+ const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>();
+ if (!FPT)
+ return;
+
+ // Verify the function takes two arguments, three in the _chk version.
+ int numArgs = FPT->getNumParams();
+ if (numArgs != 2)
+ return;
+
+ // Verify the type for both arguments.
+ for (int i = 0; i < 2; i++) {
+ // Verify that the arguments are pointers.
+ const PointerType *PT = FPT->getParamType(i)->getAs<PointerType>();
+ if (!PT)
+ return;
+
+ // Verify that the argument is a 'char*'.
+ if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().CharTy)
+ return;
+ }
+ // Second argument has to have 'const' qualifier
+ if (!FPT->getParamType(1)
+ ->getAs<PointerType>()
+ ->getPointeeType()
+ .isConstQualified())
+ return;
+
+ // Verify that the function returns 'char *'
+ const PointerType *PTReturnType = FPT->getReturnType()->getAs<PointerType>();
+ if (!PTReturnType)
+ return;
+ if (PTReturnType->getPointeeType().getUnqualifiedType() !=
+ BR.getContext().CharTy)
+ return;
+
+ const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+ *Source = CE->getArg(1)->IgnoreImpCasts();
+
+ if (const auto *Array = dyn_cast<ConstantArrayType>(Target->getType())) {
+ uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+ if (const auto *String = dyn_cast<StringLiteral>(Source)) {
+ if (ArraySize >= String->getLength() + 1)
+ return;
+ }
+ }
+
+ // Issue a warning.
+ PathDiagnosticLocation CELoc =
+ PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+ BR.EmitBasicReport(AC->getDecl(), filter.checkName_strcpy,
+ "Potential insecure memory buffer bounds restriction in "
+ "call 'lstrcpyA'",
+ "Security",
+ "Call to function 'lstrcpyA' is insecure as it does not "
+ "provide bounding of the memory buffer. Consider "
+ "using StringCchCopy instead. CWE-119.",
+ CELoc, CE->getCallee()->getSourceRange());
+}
+
//===----------------------------------------------------------------------===//
// Check: Any use of 'mktemp' is insecure. It is obsoleted by mkstemp().
// CWE-377: Insecure Temporary File
@@ -733,6 +889,60 @@ void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) {
CELoc, CE->getCallee()->getSourceRange());
}
+// TODO: Add the CWE number if it exists for this?
+//===----------------------------------------------------------------------===//
+// Check: '_strdup' should not be used.
+//
+// CWE-690: Unchecked Return Value to NULL Pointer Dereference
+//===----------------------------------------------------------------------===//
+
+void WalkAST::checkCall_strdup(const CallExpr *CE, const FunctionDecl *FD) {
+ if (!filter.check_strdup)
+ return;
+
+ // Verify that the operating system is Windows
+ if (!FD->getASTContext().getTargetInfo().getTriple().isOSWindows())
+ return;
+
+ const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>();
+ if (!FPT)
+ return;
+
+ // Verify the function takes one argument
+ int numArgs = FPT->getNumParams();
+ if (numArgs != 1)
+ return;
+
+ // Verify that the argument is a pointer
+ const PointerType *PT = FPT->getParamType(0)->getAs<PointerType>();
+ if (!PT)
+ return;
+
+ // Verify that the argument is a 'const char*'.
+ if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().CharTy)
+ return;
+ if (!PT->getPointeeType().isConstQualified())
+ return;
+
+ // Verify that the function returns 'char *'
+ const PointerType *PTReturnType = FPT->getReturnType()->getAs<PointerType>();
+ if (!PTReturnType)
+ return;
+ if (PTReturnType->getPointeeType().getUnqualifiedType() !=
+ BR.getContext().CharTy)
+ return;
+
+ // Issue a warning.
+ PathDiagnosticLocation CELoc =
+ PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+ BR.EmitBasicReport(AC->getDecl(), filter.checkName_strdup,
+ "Potential unfreed memory from call of '_strdup'",
+ "Security",
+ "Call to function '_strdup' allocates memory on heap "
+ "and requires to be freed manually. CWE-690.",
+ CELoc, CE->getCallee()->getSourceRange());
+}
+
//===----------------------------------------------------------------------===//
// Check: Any use of 'sprintf', 'vsprintf', 'scanf', 'wscanf', 'fscanf',
// 'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf',
@@ -1106,9 +1316,12 @@ REGISTER_CHECKER(bcopy)
REGISTER_CHECKER(bzero)
REGISTER_CHECKER(gets)
REGISTER_CHECKER(getpw)
+REGISTER_CHECKER(lstrcatA)
+REGISTER_CHECKER(lstrcpyA)
REGISTER_CHECKER(mkstemp)
REGISTER_CHECKER(mktemp)
REGISTER_CHECKER(strcpy)
+REGISTER_CHECKER(strdup)
REGISTER_CHECKER(rand)
REGISTER_CHECKER(vfork)
REGISTER_CHECKER(FloatLoopCounter)
``````````
</details>
https://github.com/llvm/llvm-project/pull/164183
More information about the cfe-commits
mailing list