Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -22,6 +22,7 @@ def Core : Package<"core">; def CoreBuiltin : Package<"builtin">, ParentPackage, Hidden; def CoreUninitialized : Package<"uninitialized">, ParentPackage; +def CorePointerToIntegralTypeCast : Package<"PointerToIntegralTypeCast">, ParentPackage; def CoreAlpha : Package<"core">, ParentPackage; // The OptIn package is for checkers that are not alpha and that would normally @@ -422,6 +423,13 @@ } // end "core.uninitialized" +let ParentPackage = CorePointerToIntegralTypeCast in { + +def PtrToIntegCastLibcChecker : Checker<"LibcAPI">, + HelpText<"Check for misuse of libc APIs by passing a pointer casted to its integral value">, + Documentation; +} // end "core.PointerToIntegralTypeCast" + //===----------------------------------------------------------------------===// // Unix API checkers. //===----------------------------------------------------------------------===// Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -89,6 +89,7 @@ PointerIterationChecker.cpp PointerSortingChecker.cpp PointerSubChecker.cpp + PointerToIntegralTypeCast/PtrToIntegCastLibcChecker.cpp PthreadLockChecker.cpp cert/PutenvWithAutoChecker.cpp RetainCountChecker/RetainCountChecker.cpp Index: clang/lib/StaticAnalyzer/Checkers/PointerToIntegralTypeCast/PtrToIntegCastLibcChecker.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/PointerToIntegralTypeCast/PtrToIntegCastLibcChecker.cpp @@ -0,0 +1,258 @@ +//== CStringSyntaxChecker.cpp - CoreFoundation containers API *- C++ -*-==// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// An AST checker that looks for common pitfalls when using C string APIs. +// - Identifies erroneous patterns in the last argument to strncat - the number +// of bytes to copy. +// +//===----------------------------------------------------------------------===// +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/AST/Expr.h" +#include "clang/AST/OperationKinds.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/Analysis/AnalysisDeclContext.h" +#include "clang/Basic/TargetInfo.h" +#include "clang/Basic/TypeTraits.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/Support/raw_ostream.h" +#include + +using namespace clang; +using namespace ento; + +namespace { + +class FindMemAddrUsedAsSize: public StmtVisitor { + const CheckerBase *Checker; + BugReporter &BR; + AnalysisDeclContext* AC; +public: + FindMemAddrUsedAsSize(const CheckerBase *Checker, BugReporter &BR, AnalysisDeclContext *AC) + : Checker(Checker), BR(BR), AC(AC) {} + + // This is just traversing. + void VisitChildren(Stmt *S) { + for (Stmt *Child : S->children()) + if (Child) + Visit(Child); + } + void VisitStmt(Stmt *S) { + VisitChildren(S); + } + + // The business part. + void VisitCallExpr(CallExpr *CE); +}; + +void FindMemAddrUsedAsSize::VisitCallExpr(CallExpr *CE) { + const FunctionDecl *FD = CE->getDirectCallee(); + if (!FD) + return; + + // FIXME: We can skip some arithmetic expressions too. + // FIXME: The only class of arithmetic expression that seems legal is address subtraction (seems like a legal way how to find length of a buffer). + // FIXME: + * / seem illegal in 100% cases. + // FIXME: Subtraction of a constant also seem illegal. Subtraction of a variable has false positives - e. g. there's an address stored in the variable. + // FIXME: A weird (but rare?) false positive would also be nullptr converted to int. + auto SkipCastsToPtrToIntCast = [](const Expr *E) -> const CastExpr * { + // Example: strlen() address passed as lenght + // `-ImplicitCastExpr 'unsigned long (*)(const char *)' part_of_explicit_cast + // `-DeclRefExpr 'unsigned long (const char *)' Function 'strlen' 'unsigned long (const char *)' + while (E) { + const CastExpr * Cast = dyn_cast(E); + if (Cast) { + if (Cast->getCastKind() == CK_PointerToIntegral) { + return Cast; + } + E = Cast->getSubExpr(); + continue; + } + // We don't want to skip any other nodes. + break; + } + return nullptr; + }; + + auto NameIs = [&FD](llvm::StringRef name) { + return CheckerContext::isCLibraryFunction(FD, name); + }; + + // Parameters of specific functions which semantics won't match memory address value. + llvm::DenseSet ArgsOfInterest; + // FIXME: We could further decreate the number of false positives by checking function signature. + if ( + NameIs("feclearexcept") + || NameIs("feraiseexcept") + || NameIs("fesetround") + || NameIs("fetestexcept") + || NameIs("setlocale") + || NameIs("ceil") + || NameIs("floor") + || NameIs("trunc") + || NameIs("round") + || NameIs("lround") + || NameIs("llround") + || NameIs("rint") + || NameIs("lrint") + || NameIs("llrint") + || NameIs("nearbyint") + || NameIs("fdim") + || NameIs("fmax") + || NameIs("fmin") + || NameIs("fabs") + || NameIs("abs") + || NameIs("fma") + || NameIs("fputc") + || NameIs("putc") + || NameIs("putchar") + || NameIs("ungetc") + || NameIs("malloc") + || NameIs("labs") + || NameIs("llabs") + || NameIs("fputwc") + || NameIs("putwc") + || NameIs("putwchar") + || NameIs("ungetwc") + || NameIs("btowc") + || NameIs("iswcntrl") + || NameIs("iswdigit") + || NameIs("iswgraph") + || NameIs("iswlower") + || NameIs("iswprint") + || NameIs("iswpunct") + || NameIs("iswspace") + || NameIs("iswupper") + || NameIs("iswxdigit") + || NameIs("towlower") + || NameIs("towupper") + || NameIs("iswctype") + || NameIs("towctrans") + ) { + ArgsOfInterest.insert(CE->getArg(0)); + } else if ( + NameIs("fegetexceptflag") + || NameIs("fesetexceptflag") + || NameIs("longjmp") + || NameIs("raise") + || NameIs("snprintf") + || NameIs("vsnprintf") + || NameIs("fgets") + || NameIs("realloc") + || NameIs("mblen") + || NameIs("wctomb") + || NameIs("strftime") + || NameIs("c16rtomb") + || NameIs("c32rtomb") + || NameIs("fgetws") + || NameIs("fwide") + || NameIs("swprintf") + || NameIs("vswprintf") + || NameIs("mbrlen") + || NameIs("wcrtomb") + || NameIs("wcschr") + || NameIs("wcsrchr") + || NameIs("wcsftime") + ) { + ArgsOfInterest.insert(CE->getArg(1)); + } else if ( + NameIs("strtol") + || NameIs("strtoll") + || NameIs("strtoul") + || NameIs("strtoull") + || NameIs("mbtowc") + || NameIs("mbstowcs") + || NameIs("wcstombs") + || NameIs("memcpy") + || NameIs("memmove") + || NameIs("strncpy") + || NameIs("mbrtoc16") + || NameIs("mbrtoc32") + || NameIs("wcstol") + || NameIs("wcstoll") + || NameIs("wcstoul") + || NameIs("wcstoull") + || NameIs("mbrtowc") + || NameIs("mbsrtowcs") + || NameIs("wcsrtombs") + || NameIs("wcsncat") + || NameIs("wcsncmp") + || NameIs("wcsncpy") + || NameIs("wcsxfrm") + || NameIs("wmemcmp") + || NameIs("wmemcpy") + || NameIs("wmemmove") + ) { + ArgsOfInterest.insert(CE->getArg(2)); + } else if (NameIs("calloc")) { + ArgsOfInterest.insert(CE->getArg(0)); + ArgsOfInterest.insert(CE->getArg(1)); + } else if ( + NameIs("fread") + || NameIs("fwrite") + || NameIs("fseek") + || NameIs("qsort") + || NameIs("wmemchr") + || NameIs("wmemset") + ) { + ArgsOfInterest.insert(CE->getArg(1)); + ArgsOfInterest.insert(CE->getArg(2)); + } else if ( + NameIs("bsearch") + || NameIs("setvbuf") + ) { + ArgsOfInterest.insert(CE->getArg(2)); + ArgsOfInterest.insert(CE->getArg(3)); + } + + for (const Expr* Arg : ArgsOfInterest) { + const CastExpr *PtrToIntCast = SkipCastsToPtrToIntCast(Arg); + + if (PtrToIntCast) { + PathDiagnosticLocation Loc = + PathDiagnosticLocation::createBegin(PtrToIntCast, BR.getSourceManager(), AC); + + SmallString<256> S; + llvm::raw_svector_ostream os(S); + os << "Memory address is used as an integer in context where it's likely to be a bug."; + // TODO get name & type of a DeclRefExpr from LenArg (past PtrToIntCast) + + BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument", + "C String API", os.str(), Loc, + Arg->getSourceRange()); + } + } + + // Recurse and check children. + VisitStmt(CE); +} + +} // end anonymous namespace + +namespace { +class PtrToIntegCastLibcChecker : public Checker { +public: + + void checkASTCodeBody(const Decl *D, AnalysisManager& Mgr, + BugReporter &BR) const { + FindMemAddrUsedAsSize walker(this, BR, Mgr.getAnalysisDeclContext(D)); + walker.Visit(D->getBody()); + } +}; +} + +void ento::registerPtrToIntegCastLibcChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} + +bool ento::shouldRegisterPtrToIntegCastLibcChecker(const CheckerManager &mgr) { + return true; +} Index: clang/test/Analysis/Checkers/PointerToIntegralTypeCast/PtrToIntegCastLibcChecker.c =================================================================== --- /dev/null +++ clang/test/Analysis/Checkers/PointerToIntegralTypeCast/PtrToIntegCastLibcChecker.c @@ -0,0 +1,31 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core.PointerToIntegralTypeCast.LibcAPI -verify %s + +#define NULL 0 +typedef unsigned long size_t; + +typedef void* FILE; +int putc(int c, FILE *stream); +void putc_misuse(void) { + FILE* file = NULL; + int* a_random_ptr = NULL; + putc((size_t) a_random_ptr, file); // expected-warning {{.*}} + // TODO proper expected +} + +void *memmove(void *dest, const void *src, size_t n); +size_t get_size_of_buffer(int* buffer) { /* real impl would be here */ return 0; } +void memmove_misuse(void) { + int dest[10]; + int* src = NULL; + memmove(dest, src, (size_t) get_size_of_buffer); // expected-warning {{.*}} + // TODO proper expected +} + +void *memcpy(void *dest, const void *src, size_t n); +void memcpy_misuse() { + int dest[10]; + int* src = NULL; + size_t buffer_sizes[1]; + memcpy(dest, src, (size_t) buffer_sizes); // expected-warning {{.*}} + // TODO proper expected +} \ No newline at end of file