https://github.com/Ruhin575 created https://github.com/llvm/llvm-project/pull/234582
Extended the security.insecureAPI.strcpy checker to detect six more unsafe string functions: lstrcpyA, lstrcatA, lstrcpyW, lstrcatW, wcscpy, and wcscat. Partial fix for #103038. Changes • Added the six functions to the existing checker. • Updated type checking to support both char* and wchar_t*, including typedefs. • Updated warning messages to show the actual function name. • Changed the safe-copy check to compare array element counts instead of bytes. • Added tests for all six functions, including safe and unsafe wide-character copies. Testing • Built Clang successfully using ninja -C build clang. • Ran security-syntax-checks.m using LLVM lit. All 4 test configurations passed. • Tested warnings for all six newly added functions. • Tested safe and unsafe string copies, including wide-character buffers of different sizes. • Checked formatting using git-clang-format and whitespace using git diff --check. >From 39b974bdb1a67c898b6c190cacbd2b0b5e19fc1b Mon Sep 17 00:00:00 2001 From: Ruhin575 <[email protected]> Date: Thu, 22 Oct 2026 15:53:40 +0530 Subject: [PATCH] [clang][analyzer] Detect additional unsafe string functions Extend the security.insecureAPI.strcpy checker to detect unbounded string copy and concatenation functions: lstrcpyA, lstrcatA, lstrcpyW, lstrcatW, wcscpy, and wcscat. Add the new function names to the dispatch table and extend checkCall_strCommon to accept both char* and wchar_t* parameters. Use canonical types to handle typedefs and verify that both parameters have matching character types. Update diagnostic messages to report the actual function name and provide appropriate suggestions for narrow and wide strings. Preserve existing diagnostics for fortified variants. Update the safe-copy check to compare array element counts rather than byte sizes, and add regression tests for the new functions and wide-character buffer-size cases. Partial fix for #103038. --- .../Checkers/CheckSecuritySyntaxOnly.cpp | 134 ++++++++++++++---- clang/test/Analysis/security-syntax-checks.m | 60 ++++++++ 2 files changed, 164 insertions(+), 30 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 797e1efc7e8ca5..3e46f6394e81a7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -163,8 +163,10 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { .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) + .Cases({"strcpy", "__strcpy_chk", "lstrcpyA", "wcscpy", "lstrcpyW"}, + &WalkAST::checkCall_strcpy) + .Cases({"strcat", "__strcat_chk", "lstrcatA", "wcscat", "lstrcatW"}, + &WalkAST::checkCall_strcat) .Cases({"sprintf", "vsprintf", "scanf", "wscanf", "fscanf", "fwscanf", "vscanf", "vwscanf", "vfscanf", "vfwscanf"}, &WalkAST::checkDeprecatedOrUnsafeBufferHandling) @@ -689,29 +691,68 @@ void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) { if (!checkCall_strCommon(CE, FD)) return; - const auto *Target = CE->getArg(0)->IgnoreImpCasts(), - *Source = CE->getArg(1)->IgnoreImpCasts(); + const auto *Target = CE->getArg(0)->IgnoreImpCasts(); + const auto *Source = CE->getArg(1)->IgnoreImpCasts(); + // Check whether a string literal fits into the destination array. if (const auto *Array = dyn_cast<ConstantArrayType>(Target->getType())) { - uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8; + uint64_t ArraySize = Array->getSize().getZExtValue(); + if (const auto *String = dyn_cast<StringLiteral>(Source)) { - if (ArraySize >= String->getLength() + 1) + ASTContext &Ctx = BR.getContext(); + + QualType DestElementType = + Ctx.getCanonicalType(Array->getElementType()).getUnqualifiedType(); + + QualType SrcElementType = + Ctx.getCanonicalType( + String->getType()->getAsArrayTypeUnsafe()->getElementType()) + .getUnqualifiedType(); + + if (DestElementType == SrcElementType && + ArraySize >= String->getLength() + 1) return; } } + // Get the function name and normalize fortified variants. + StringRef Name = FD->getIdentifier()->getName(); + Name.consume_front("__builtin_"); + + if (Name == "__strcpy_chk") + Name = "strcpy"; + + bool IsWide = Name == "wcscpy" || Name == "lstrcpyW"; + + // Construct the warning title and description. + SmallString<128> Buf1; + SmallString<512> Buf2; + llvm::raw_svector_ostream Out1(Buf1); + llvm::raw_svector_ostream Out2(Buf2); + + Out1 << "Potential insecure memory buffer bounds restriction in call '" + << Name << "'"; + + Out2 << "Call to function '" << Name + << "' is insecure as it does not " + "provide bounding of the memory buffer. Replace " + "unbounded copy functions with analogous functions that " + "support length arguments"; + + if (IsWide) + Out2 << " for wide-character strings"; + else + Out2 << " such as 'strlcpy'"; + + Out2 << ". CWE-119."; + // 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 'strcpy'", - "Security", - "Call to function 'strcpy' is insecure as it does not " - "provide bounding of the memory buffer. Replace " - "unbounded copy functions with analogous functions that " - "support length arguments such as 'strlcpy'. CWE-119.", - CELoc, CE->getCallee()->getSourceRange()); + PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); + + BR.EmitBasicReport(AC->getDecl(), filter.checkName_strcpy, Out1.str(), + "Security", Out2.str(), CELoc, + CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// @@ -728,18 +769,38 @@ void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) { if (!checkCall_strCommon(CE, FD)) return; + // Get the function name and normalize fortified variants. + StringRef Name = FD->getIdentifier()->getName(); + Name.consume_front("__builtin_"); + if (Name == "__strcat_chk") + Name = "strcat"; + bool IsWide = Name == "wcscat" || Name == "lstrcatW"; + // Construct the warning title and description. + SmallString<128> Buf1; + SmallString<512> Buf2; + llvm::raw_svector_ostream Out1(Buf1); + llvm::raw_svector_ostream Out2(Buf2); + + Out1 << "Potential insecure memory buffer bounds restriction in call '" + << Name << "'"; + + Out2 << "Call to function '" << Name + << "' is insecure as it does not " + "provide bounding of the memory buffer. Replace " + "unbounded copy functions with analogous functions that " + "support length arguments"; + if (IsWide) + Out2 << " for wide-character strings"; + else + Out2 << " such as 'strlcat'"; + Out2 << ". CWE-119."; + // 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 'strcat'", - "Security", - "Call to function 'strcat' is insecure as it does not " - "provide bounding of the memory buffer. Replace " - "unbounded copy functions with analogous functions that " - "support length arguments such as 'strlcat'. CWE-119.", - CELoc, CE->getCallee()->getSourceRange()); + PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); + BR.EmitBasicReport(AC->getDecl(), filter.checkName_strcpy, Out1.str(), + "Security", Out2.str(), CELoc, + CE->getCallee()->getSourceRange()); } //===----------------------------------------------------------------------===// @@ -838,15 +899,28 @@ bool WalkAST::checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD) { if (numArgs != 2 && numArgs != 3) return false; - // Verify the type for both arguments. + ASTContext &Ctx = BR.getContext(); + QualType CharType = Ctx.getCanonicalType(Ctx.CharTy).getUnqualifiedType(); + QualType WideCharType = + Ctx.getCanonicalType(Ctx.getWideCharType()).getUnqualifiedType(); + + QualType FirstPointeeType; + + // Verify that both arguments are pointers to the same character type. for (int i = 0; i < 2; i++) { - // Verify that the arguments are pointers. const PointerType *PT = FPT->getParamType(i)->getAs<PointerType>(); if (!PT) return false; - // Verify that the argument is a 'char*'. - if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().CharTy) + QualType PointeeType = + Ctx.getCanonicalType(PT->getPointeeType()).getUnqualifiedType(); + + if (PointeeType != CharType && PointeeType != WideCharType) + return false; + + if (i == 0) + FirstPointeeType = PointeeType; + else if (PointeeType != FirstPointeeType) return false; } diff --git a/clang/test/Analysis/security-syntax-checks.m b/clang/test/Analysis/security-syntax-checks.m index 154f0c1bae427c..37417c0f2807f4 100644 --- a/clang/test/Analysis/security-syntax-checks.m +++ b/clang/test/Analysis/security-syntax-checks.m @@ -217,6 +217,66 @@ void test_strcat(void) { strcat(x, y); //expected-warning{{Call to function 'strcat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcat'. CWE-119}} } +// Windows-specific string functions +char *lstrcpyA(char *dst, const char *src); +char *lstrcatA(char *dst, const char *src); + +void test_lstrcpyA(void) { + char dst[4]; + char *src; + lstrcpyA(dst, src); // expected-warning{{Call to function 'lstrcpyA' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119}} +} + +void test_lstrcpyA_safe(void) { + char dst[10]; + lstrcpyA(dst, "hello"); // no-warning +} + +void test_lstrcatA(void) { + char dst[4] = ""; + char *src; + lstrcatA(dst, src); // expected-warning{{Call to function 'lstrcatA' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcat'. CWE-119}} +} + +// Wide-character string functions. + +typedef __WCHAR_TYPE__ test_wchar_t; + +test_wchar_t *wcscpy(test_wchar_t *dst, const test_wchar_t *src); +test_wchar_t *wcscat(test_wchar_t *dst, const test_wchar_t *src); +test_wchar_t *lstrcpyW(test_wchar_t *dst, const test_wchar_t *src); +test_wchar_t *lstrcatW(test_wchar_t *dst, const test_wchar_t *src); + +void test_wcscpy(const test_wchar_t *src) { + test_wchar_t dst[4]; + wcscpy(dst, src); // expected-warning{{Call to function 'wcscpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments for wide-character strings. CWE-119}} +} + +void test_wcscpy_small_buffer(void) { + test_wchar_t dst[4]; + wcscpy(dst, L"hello"); // expected-warning{{Call to function 'wcscpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments for wide-character strings. CWE-119}} +} + +void test_wcscpy_large_buffer(void) { + test_wchar_t dst[10]; + wcscpy(dst, L"hello"); +} + +void test_wcscat(const test_wchar_t *src) { + test_wchar_t dst[4] = {0}; + wcscat(dst, src); // expected-warning{{Call to function 'wcscat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments for wide-character strings. CWE-119}} +} + +void test_lstrcpyW(const test_wchar_t *src) { + test_wchar_t dst[4]; + lstrcpyW(dst, src); // expected-warning{{Call to function 'lstrcpyW' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments for wide-character strings. CWE-119}} +} + +void test_lstrcatW(const test_wchar_t *src) { + test_wchar_t dst[4] = {0}; + lstrcatW(dst, src); // expected-warning{{Call to function 'lstrcatW' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments for wide-character strings. CWE-119}} +} + //===----------------------------------------------------------------------=== // vfork() //===----------------------------------------------------------------------=== _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
