Add bugprone-sprintf-argument-overlap

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -63,6 +63,7 @@
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
+#include "SprintfArgumentOverlapCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
 #include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
@@ -204,6 +205,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-sizeof-container");
     CheckFactories.registerCheck<SizeofExpressionCheck>(
         "bugprone-sizeof-expression");
+    CheckFactories.registerCheck<SprintfArgumentOverlapCheck>(
+        "bugprone-sprintf-argument-overlap");
     CheckFactories.registerCheck<SpuriouslyWakeUpFunctionsCheck>(
         "bugprone-spuriously-wake-up-functions");
     CheckFactories.registerCheck<StandaloneEmptyCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -60,6 +60,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
+  SprintfArgumentOverlapCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
   StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp

clang-tools-extra/clang-tidy/bugprone/SprintfArgumentOverlapCheck.cpp

@@ -0,0 +1,106 @@
+//===--- SprintfArgumentOverlapCheck.cpp - clang-tidy --------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "SprintfArgumentOverlapCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+// Similar to forEachArgumentWithParam. forEachArgumentWithParam does not work
+// with variadic functions like sprintf, since there is no `decl()` to match
+// against in the parameter list `...`.
+AST_MATCHER_P(CallExpr, forEachArgument, ast_matchers::internal::Matcher<Expr>,
+              ArgMatcher) {
+  using namespace clang::ast_matchers::internal;
+  BoundNodesTreeBuilder Result;
+  int ParamIndex = 0;
+  bool Matched = false;
+  for (unsigned ArgIndex = 0; ArgIndex < Node.getNumArgs(); ++ArgIndex) {
+    BoundNodesTreeBuilder ArgMatches(*Builder);
+    if (ArgMatcher.matches(*(Node.getArg(ArgIndex)->IgnoreParenCasts()), Finder,
+                           &ArgMatches)) {
+      BoundNodesTreeBuilder ParamMatches(ArgMatches);
+      Result.addMatch(ArgMatches);
+      Matched = true;
+    }
+    ++ParamIndex;
+  }
+  *Builder = std::move(Result);
+  return Matched;
+}
+
+AST_MATCHER_P(Stmt, identicalTo, std::string, ID) {
+  return Builder->removeBindings(
+      [this, &Node,
+       &Finder](const ast_matchers::internal::BoundNodesMap &Nodes) {
+        const DynTypedNode &BN = Nodes.getNode(ID);
+        if (const auto *BoundStmt = BN.get<Stmt>())
+          return !utils::areStatementsIdentical(&Node, BoundStmt,
+                                                Finder->getASTContext());
+        return true;
+      });
+}
+
+AST_MATCHER(Expr, hasSideEffects) {
+  return Node.HasSideEffects(Finder->getASTContext());
+}
+
+SprintfArgumentOverlapCheck::SprintfArgumentOverlapCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      SprintfRegex(Options.get("SprintfFunction", "(::std)?::sn?printf")) {}
+
+void SprintfArgumentOverlapCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(
+          callee(functionDecl(matchesName(SprintfRegex)).bind("decl")),
+          hasArgument(0, expr(unless(hasSideEffects())).bind("firstArgExpr")),
+          forEachArgument(expr(unless(equalsBoundNode("firstArgExpr")),
+                               identicalTo("firstArgExpr"))
+                              .bind("otherArgExpr")))
+          .bind("call"),
+      this);
+}
+
+void SprintfArgumentOverlapCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *FirstArg = Result.Nodes.getNodeAs<Expr>("firstArgExpr");
+  const auto *OtherArg = Result.Nodes.getNodeAs<Expr>("otherArgExpr");
+  const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");
+  const auto *FnDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl");
+
+  if (!FirstArg || !OtherArg || !Call || !FnDecl)
+    return;
+
+  std::optional<unsigned> ArgIndex;
+  for (unsigned I = 0; I != Call->getNumArgs(); ++I) {
+    if (Call->getArg(I)->IgnoreUnlessSpelledInSource() == OtherArg) {
+      ArgIndex = I;
+      break;
+    }
+  }
+  if (!ArgIndex)
+    return;
+
+  diag(OtherArg->getBeginLoc(),
+       "the %ordinal0 argument in %1 overlaps the 1st argument, "
+       "which is undefined behavior")
+      << (*ArgIndex + 1) << FnDecl << FirstArg->getSourceRange()
+      << OtherArg->getSourceRange();
+}
+
+void SprintfArgumentOverlapCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SprintfRegex", SprintfRegex);
+}
+
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/SprintfArgumentOverlapCheck.h

@@ -0,0 +1,40 @@
+//===--- SprintfArgumentOverlapCheck.h - clang-tidy -------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SPRINTFARGUMENTOVERLAPCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SPRINTFARGUMENTOVERLAPCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Warns if any arguments to the ``sprintf`` family of functions overlap with
+/// the destination buffer (the first argument).
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/sprintf-argument-overlap.html
+class SprintfArgumentOverlapCheck : public ClangTidyCheck {
+public:
+  SprintfArgumentOverlapCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus || LangOpts.C99;
+  }
+
+private:
+  const std::string SprintfRegex;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SPRINTFARGUMENTOVERLAPCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -151,6 +151,12 @@ New checks
 
   Finds nondeterministic usages of pointers in unordered containers.
 
+- New :doc:`bugprone-sprintf-argument-overlap
+  <clang-tidy/checks/bugprone/sprintf-argument-overlap>` check.
+
+  Warns if any arguments to the ``sprintf`` family of functions overlap with the
+  first argument.
+
 - New :doc:`bugprone-tagged-union-member-count
   <clang-tidy/checks/bugprone/tagged-union-member-count>` check.
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-argument-overlap.rst

@@ -0,0 +1,36 @@
+.. title:: clang-tidy - bugprone-sprintf-argument-overlap
+
+bugprone-sprintf-argument-overlap
+=================================
+
+Warns if any arguments to the ``sprintf`` family of functions overlap with the
+destination buffer (the first argument).
+
+.. code-block:: c++
+
+    char buf[20] = {"hi"};
+    sprintf(buf, "%s%d", buf, 0);
+
+If copying takes place between objects that overlap, the behavior is undefined.
+This is stated in the `C23/N3220 standard
+<https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf>`_
+(sections 7.23.6.5 and 7.23.6.6), as well as the `POSIX.1-2024 standard
+<https://pubs.opengroup.org/onlinepubs/9799919799/>`_.
+
+In practice, passing the output buffer to an input argument can result in
+incorrect output. For example, Linux with glibc may produce the following.
+
+.. code-block:: c++
+
+    char buf[10];
+    sprintf(buf, "%s", "12");
+    sprintf(buf, "%s%s", "34", buf);
+    printf("%s\n", buf); // prints 3434
+
+Options
+-------
+
+.. option:: SprintfRegex
+
+   A regex specifying the ``sprintf`` family of functions to match on. By default,
+   this is `(::std)?::sn?printf`.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -131,6 +131,7 @@ Clang-Tidy Checks
    :doc:`bugprone-signed-char-misuse <bugprone/signed-char-misuse>`,
    :doc:`bugprone-sizeof-container <bugprone/sizeof-container>`,
    :doc:`bugprone-sizeof-expression <bugprone/sizeof-expression>`,
+   :doc:`bugprone-sprintf-argument-overlap <bugprone/sprintf-argument-overlap>`,
    :doc:`bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions>`,
    :doc:`bugprone-standalone-empty <bugprone/standalone-empty>`, "Yes"
    :doc:`bugprone-string-constructor <bugprone/string-constructor>`, "Yes"

clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-argument-overlap.cpp

@@ -0,0 +1,86 @@
+// RUN: %check_clang_tidy %s bugprone-sprintf-argument-overlap %t
+
+using size_t = decltype(sizeof(int));
+
+extern "C" int sprintf(char *s, const char *format, ...);
+extern "C" int snprintf(char *s, size_t n, const char *format, ...);
+
+namespace std {
+  int snprintf(char *s, size_t n, const char *format, ...);
+}
+
+struct st_t {
+  char buf[10];
+  char buf2[10];
+};
+
+struct st2_t {
+  st_t inner;
+};
+
+struct st3_t {
+  st2_t inner;
+};
+
+void first_arg_overlaps() {
+  char buf[10];
+  sprintf(buf, "%s", buf);
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  snprintf(buf, sizeof(buf), "%s", buf);
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: the 4th argument in 'snprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  std::snprintf(buf, sizeof(buf), "%s", buf);
+  // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: the 4th argument in 'snprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  sprintf(buf+1, "%s", (buf+1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  sprintf(buf+1, "%s", buf+2);
+  sprintf(buf+1, "%s", buf[1]);
+
+  char* c = &buf[0];
+  sprintf(c, "%s", c);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  snprintf(c, sizeof(buf), "%s", c);
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: the 4th argument in 'snprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+
+  snprintf(c, sizeof(buf), "%s%s", c, c);
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: the 4th argument in 'snprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  // CHECK-MESSAGES: :[[@LINE-2]]:39: warning: the 5th argument in 'snprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+
+  char buf2[10];
+  sprintf(buf, "%s", buf2);
+  sprintf(buf, "%s", buf2, buf);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: the 4th argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+
+  st_t st1, st2;
+  sprintf(st1.buf, "%s", st1.buf);
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  sprintf(st1.buf, "%s", st1.buf2);
+  sprintf(st1.buf, "%s", st2.buf);
+
+  st3_t st3;
+  sprintf(st3.inner.inner.buf, "%s", st3.inner.inner.buf);
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  sprintf((st3.inner.inner.buf), "%s", st3.inner.inner.buf);
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+
+  st_t* stp;
+  sprintf(stp->buf, "%s", stp->buf);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  sprintf((stp->buf), "%s", stp->buf);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  stp = &st1;
+  sprintf(stp->buf, "%s", st1.buf);
+
+  char bufs[10][10];
+  sprintf(bufs[1], "%s", bufs[1]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:26:  warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+  sprintf(bufs[0], "%s", bufs[1]);
+
+  char bufss[10][10][10];
+  sprintf(bufss[0][1], "%s", bufss[0][1]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:30:  warning: the 3rd argument in 'sprintf' overlaps the 1st argument, which is undefined behavior [bugprone-sprintf-argument-overlap]
+
+  sprintf(bufss[0][0], "%s", bufss[0][1]);
+
+  int i = 0;
+  sprintf(bufss[0][++i], "%s", bufss[0][++i]);
+}