================
@@ -682,38 +687,86 @@ void WalkAST::checkCall_mkstemp(const CallExpr *CE, const
FunctionDecl *FD) {
// the Bounds of a Memory Buffer
//===----------------------------------------------------------------------===//
+
+void WalkAST::emitUnboundedStringDiagnostic(const CallExpr *CE, StringRef Name,
+ StringRef NarrowReplacement,
+ bool IsWide) {
+ // 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 '" << NarrowReplacement << "'";
+
+ Out2 << ". CWE-119.";
+
+ // Issue a warning.
+ PathDiagnosticLocation CELoc =
+ PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+
+ BR.EmitBasicReport(AC->getDecl(), filter.checkName_strcpy, Out1.str(),
+ "Security", Out2.str(), CELoc,
+ CE->getCallee()->getSourceRange());
+}
+
+
void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
if (!filter.check_strcpy)
return;
- if (!checkCall_strCommon(CE, FD))
+ // 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";
+
+ if (!checkCall_strCommon(CE, FD, IsWide))
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;
}
}
- // 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());
+ emitUnboundedStringDiagnostic(CE, Name, "strlcpy", IsWide);
}
+
----------------
benedekaibas wrote:
```suggestion
```
nit: At _most_ places in this checker there are only one spaces between the
functions and the comments, but at a few places like this one there are two
spaces. I'd advise removing this, but no hard preference.
https://github.com/llvm/llvm-project/pull/234582
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
