etienneb created this revision.
etienneb added a reviewer: alexfh.
etienneb added a subscriber: cfe-commits.

This patch add the support for detecting suspicious string
literals and their usage.

The following example shows a incorrect character escaping leading 
to an embedded NUL character. 
```
  std::string str = "\0x42";   // Should be "\x42".
```

The patch also add detection of truncated literal when a literal
is passed to a string constuctor.

http://reviews.llvm.org/D18783

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/StringLiteralWithEmbeddedNulCheck.cpp
  clang-tidy/misc/StringLiteralWithEmbeddedNulCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-string-literal-with-embedded-nul.rst
  test/clang-tidy/misc-string-literal-with-embedded-nul.cpp

Index: test/clang-tidy/misc-string-literal-with-embedded-nul.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-string-literal-with-embedded-nul.cpp
@@ -0,0 +1,86 @@
+// RUN: %check_clang_tidy %s misc-string-literal-with-embedded-nul %t
+
+namespace std {
+template <typename T>
+class allocator {};
+template <typename T>
+class char_traits {};
+template <typename C, typename T, typename A>
+struct basic_string {
+  typedef basic_string<C, T, A> _Type;
+  basic_string();
+  basic_string(const C *p, const A &a = A());
+
+  _Type& operator+=(const C* s);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string<char, std::char_traits<char>, std::allocator<char>> string;
+typedef basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t>> wstring;
+}
+
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+
+const char Valid[] = "This is valid \x12.";
+const char Strange[] = "This is strange \0x12 and must be fixed";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: suspicious embedded NUL character [misc-string-literal-with-embedded-nul]
+
+const char textA[] = "\0x01\0x02\0x03\0x04";
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious embedded NUL character
+const wchar_t textW[] = L"\0x01\0x02\0x03\0x04";
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: suspicious embedded NUL character
+
+const char A[] = "\0";
+const char B[] = "\0x";
+const char C[] = "\0x1";
+const char D[] = "\0x11";
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: suspicious embedded NUL character
+
+const wchar_t E[] = L"\0";
+const wchar_t F[] = L"\0x";
+const wchar_t G[] = L"\0x1";
+const wchar_t H[] = L"\0x11";
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: suspicious embedded NUL character
+
+const char I[] = "\000\000\000\000";
+const char J[] = "\0\0\0\0\0\0";
+const char K[] = "";
+
+const char L[] = "\0x12" "\0x12" "\0x12" "\0x12";
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: suspicious embedded NUL character
+
+void TestA() {
+  std::string str1 = "abc\0def";
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: truncated string literal
+  std::string str2 = "\0";
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: truncated string literal
+  std::string str3("\0");
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: truncated string literal
+  std::string str4{"\x00\x01\x02\x03"};
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: truncated string literal
+
+  std::string str;
+  str += "abc\0def";
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: truncated string literal
+  str = "abc\0def";
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: truncated string literal
+
+  if (str == "abc\0def") return;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: truncated string literal
+  if ("abc\0def" == str) return;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: truncated string literal
+}
+
+void TestW() {
+  std::wstring str1 = L"abc\0def";
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: truncated string literal
+  std::wstring str2 = L"\0";
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: truncated string literal
+  std::wstring str3(L"\0");
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: truncated string literal
+  std::wstring str4{L"\x00\x01\x02\x03"};
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: truncated string literal
+}
+
Index: docs/clang-tidy/checks/misc-string-literal-with-embedded-nul.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-string-literal-with-embedded-nul.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - misc-string-literal-with-embedded-nul
+
+misc-string-literal-with-embedded-nul
+=====================================
+
+Find occurences of string literal with embedded NUL character and validate
+their usage.
+
+
+Invalid escaping
+^^^^^^^^^^^^^^^^
+
+Special characters can be escaped within a string literal by using their
+hexadecimal encoding like ``\x42``. A common mistake is to escape them
+like this ``\0x42`` where the ``\0`` stand for the NUL character.
+
+.. code:: c++
+
+  const char* Example[] = "Invalid character: \0x12 should be \x12";
+  const char* Bytes[] = "\x03\0x02\0x01\0x00\0xFF\0xFF\0xFF";
+
+
+Truncated literal
+^^^^^^^^^^^^^^^^^
+
+String-like classes can manipulate strings with embedded NUL as they are
+keeping track of the bytes and the length. This is not the case for an
+``char*`` (NUL-terminated) string.
+
+A common mistake is to pass a string-literal (NUL-terminated) to a string
+constructor. The bytes after the first NUL character are truncated.
+
+.. code:: c++
+
+  std::string str("abc\0def");  // "def" is truncated
+  str += "\0";                  // This statement is doing nothing
+  if (str == "\0abc") return;   // This expression is always true
+
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,9 +31,9 @@
    google-build-using-namespace
    google-explicit-constructor
    google-global-names-in-headers
-   google-readability-braces-around-statements (redirects to readability-braces-around-statements) <readability-braces-around-statements>
+   google-readability-braces-around-statements (redirects to readability-braces-around-statements) <google-readability-braces-around-statements>
    google-readability-casting
-   google-readability-function-size (redirects to readability-function-size) <readability-function-size>
+   google-readability-function-size (redirects to readability-function-size) <google-readability-function-size>
    google-readability-namespace-comments
    google-readability-redundant-smartptr-get
    google-readability-todo
@@ -66,6 +66,7 @@
    misc-sizeof-container
    misc-static-assert
    misc-string-integer-assignment
+   misc-string-literal-with-embedded-nul
    misc-suspicious-missing-comma
    misc-suspicious-semicolon
    misc-swapped-arguments
Index: clang-tidy/misc/StringLiteralWithEmbeddedNulCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/StringLiteralWithEmbeddedNulCheck.h
@@ -0,0 +1,35 @@
+//===--- StringLiteralWithEmbeddedNulCheck.h - clang-tidy--------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_LITERAL_WITH_EMBEDDED_NUL_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_LITERAL_WITH_EMBEDDED_NUL_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find suspicious string literals with embedded NUL characters.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-string-literal-with-embedded-nul.html
+class StringLiteralWithEmbeddedNulCheck : public ClangTidyCheck {
+public:
+  StringLiteralWithEmbeddedNulCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_LITERAL_WITH_EMBEDDED_NUL_H
Index: clang-tidy/misc/StringLiteralWithEmbeddedNulCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/StringLiteralWithEmbeddedNulCheck.cpp
@@ -0,0 +1,93 @@
+//===--- StringLiteralWithEmbeddedNulCheck.cpp - clang-tidy----------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "StringLiteralWithEmbeddedNulCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+// Retrieve the character at offset |offset| of the string literal |SL|.
+static unsigned int GetCharAt(const StringLiteral *SL, size_t offset) {
+  if (offset >= SL->getLength()) return 0;
+  switch (SL->getCharByteWidth()) {
+    case 1:
+      return SL->getBytes()[offset];
+    case 2:
+      return reinterpret_cast<const uint16_t *>(SL->getBytes().data())[offset];
+    case 4:
+      return reinterpret_cast<const uint32_t *>(SL->getBytes().data())[offset];
+    default:
+      return 0;
+  }
+}
+
+AST_MATCHER(StringLiteral, containsNul) {
+  for (size_t i = 0; i < Node.getLength(); ++i)
+    if (GetCharAt(&Node, i) == 0) return true;
+  return false;
+}
+
+void StringLiteralWithEmbeddedNulCheck::registerMatchers(MatchFinder *Finder) {
+  // Match a string that contains embedded NUL character. Extra-checks are
+  // applied in |check| to find incorectly escaped characters.
+  Finder->addMatcher(stringLiteral(containsNul()).bind("strlit"), this);
+
+  // The remaining checks only apply to C++.
+  if (!getLangOpts().CPlusPlus) return;
+
+  const auto StrLitWithNul =
+      ignoringParenImpCasts(stringLiteral(containsNul()).bind("truncated"));
+
+  // Match string constructor.
+  const auto StringConstructorExpr = expr(anyOf(
+      cxxConstructExpr(argumentCountIs(1),
+                       hasDeclaration(cxxMethodDecl(hasName("basic_string")))),
+      // If present, the second argument is the alloc object which must not
+      // be present explicitly.
+      cxxConstructExpr(argumentCountIs(2),
+                       hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+                       hasArgument(1, cxxDefaultArgExpr()))));
+
+  // Detect passing a suspicious string literal to a string constructor.
+  // example: std::string str = "abc\0def";
+  Finder->addMatcher(
+      cxxConstructExpr(StringConstructorExpr, hasArgument(0, StrLitWithNul)),
+      this);
+
+  // Detect passing a suspisious string literal through an overloaded operator.
+  Finder->addMatcher(cxxOperatorCallExpr(hasAnyArgument(StrLitWithNul)), this);
+}
+
+void StringLiteralWithEmbeddedNulCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *SL = Result.Nodes.getNodeAs<StringLiteral>("strlit")) {
+    for (size_t i = 0; i < SL->getLength(); ++i) {
+      // Find a sequence of character like "\0x12".
+      if (GetCharAt(SL, i) == 0 && GetCharAt(SL, i + 1) == 'x' &&
+          isDigit(GetCharAt(SL, i + 2)) && isDigit(GetCharAt(SL, i + 3))) {
+        diag(SL->getLocStart(), "suspicious embedded NUL character");
+        break;
+      }
+    }
+  }
+
+  if (const auto *SL = Result.Nodes.getNodeAs<StringLiteral>("truncated")) {
+    diag(SL->getLocStart(),
+         "truncated string literal with embedded NUL character");
+  }
+}
+
+}  // namespace misc
+}  // namespace tidy
+}  // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -31,6 +31,7 @@
 #include "SizeofContainerCheck.h"
 #include "StaticAssertCheck.h"
 #include "StringIntegerAssignmentCheck.h"
+#include "StringLiteralWithEmbeddedNulCheck.h"
 #include "SuspiciousMissingCommaCheck.h"
 #include "SuspiciousSemicolonCheck.h"
 #include "SwappedArgumentsCheck.h"
@@ -89,6 +90,8 @@
         "misc-static-assert");
     CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
         "misc-string-integer-assignment");
+    CheckFactories.registerCheck<StringLiteralWithEmbeddedNulCheck>(
+        "misc-string-literal-with-embedded-nul");
     CheckFactories.registerCheck<SuspiciousMissingCommaCheck>(
         "misc-suspicious-missing-comma");
     CheckFactories.registerCheck<SuspiciousSemicolonCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -23,6 +23,7 @@
   SizeofContainerCheck.cpp
   StaticAssertCheck.cpp
   StringIntegerAssignmentCheck.cpp
+  StringLiteralWithEmbeddedNulCheck.cpp
   SuspiciousMissingCommaCheck.cpp
   SuspiciousSemicolonCheck.cpp
   SwappedArgumentsCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to