[clang-tools-extra] 35cca45 - Misleading bidirectional detection
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 12 02:39:00 PST 2022
Author: serge-sans-paille
Date: 2022-01-12T11:38:36+01:00
New Revision: 35cca45b09b87cc31cda35a66fc93bf4d630f8d2
URL: https://github.com/llvm/llvm-project/commit/35cca45b09b87cc31cda35a66fc93bf4d630f8d2
DIFF: https://github.com/llvm/llvm-project/commit/35cca45b09b87cc31cda35a66fc93bf4d630f8d2.diff
LOG: Misleading bidirectional detection
This patch implements detection of incomplete bidirectional sequence withing
comments and string literals within clang-tidy.
It detects the bidi part of https://www.trojansource.codes/trojan-source.pdf
Differential Revision: https://reviews.llvm.org/D112913
Added:
clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
clang-tools-extra/test/clang-tidy/checkers/misc-misleading-bidirectional.cpp
Modified:
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index 22213481619c5..9c827e632cd7e 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyMiscModule
DefinitionsInHeadersCheck.cpp
MiscTidyModule.cpp
+ MisleadingBidirectional.cpp
MisleadingIdentifier.cpp
MisplacedConstCheck.cpp
NewDeleteOverloadsCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
index 2defecc60741a..ef8a20683d39a 100644
--- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "DefinitionsInHeadersCheck.h"
+#include "MisleadingBidirectional.h"
#include "MisleadingIdentifier.h"
#include "MisplacedConstCheck.h"
#include "NewDeleteOverloadsCheck.h"
@@ -34,6 +35,8 @@ class MiscModule : public ClangTidyModule {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
"misc-definitions-in-headers");
+ CheckFactories.registerCheck<MisleadingBidirectionalCheck>(
+ "misc-misleading-bidirectional");
CheckFactories.registerCheck<MisleadingIdentifierCheck>(
"misc-misleading-identifier");
CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
diff --git a/clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp b/clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
new file mode 100644
index 0000000000000..7cd6fc338695c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
@@ -0,0 +1,139 @@
+//===--- MisleadingBidirectional.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 "MisleadingBidirectional.h"
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/Support/ConvertUTF.h"
+
+using namespace clang;
+using namespace clang::tidy::misc;
+
+static bool containsMisleadingBidi(StringRef Buffer,
+ bool HonorLineBreaks = true) {
+ const char *CurPtr = Buffer.begin();
+
+ enum BidiChar {
+ PS = 0x2029,
+ RLO = 0x202E,
+ RLE = 0x202B,
+ LRO = 0x202D,
+ LRE = 0x202A,
+ PDF = 0x202C,
+ RLI = 0x2067,
+ LRI = 0x2066,
+ FSI = 0x2068,
+ PDI = 0x2069
+ };
+
+ SmallVector<BidiChar> BidiContexts;
+
+ // Scan each character while maintaining a stack of opened bidi context.
+ // RLO/RLE/LRO/LRE all are closed by PDF while RLI LRI and FSI are closed by
+ // PDI. New lines reset the context count. Extra PDF / PDI are ignored.
+ //
+ // Warn if we end up with an unclosed context.
+ while (CurPtr < Buffer.end()) {
+ unsigned char C = *CurPtr;
+ if (isASCII(C)) {
+ ++CurPtr;
+ bool IsParagrapSep =
+ (C == 0xA || C == 0xD || (0x1C <= C && C <= 0x1E) || C == 0x85);
+ bool IsSegmentSep = (C == 0x9 || C == 0xB || C == 0x1F);
+ if (IsParagrapSep || IsSegmentSep)
+ BidiContexts.clear();
+ continue;
+ }
+ llvm::UTF32 CodePoint;
+ llvm::ConversionResult Result = llvm::convertUTF8Sequence(
+ (const llvm::UTF8 **)&CurPtr, (const llvm::UTF8 *)Buffer.end(),
+ &CodePoint, llvm::strictConversion);
+
+ // If conversion fails, utf-8 is designed so that we can just try next char.
+ if (Result != llvm::conversionOK) {
+ ++CurPtr;
+ continue;
+ }
+
+ // Open a PDF context.
+ if (CodePoint == RLO || CodePoint == RLE || CodePoint == LRO ||
+ CodePoint == LRE)
+ BidiContexts.push_back(PDF);
+ // Close PDF Context.
+ else if (CodePoint == PDF) {
+ if (!BidiContexts.empty() && BidiContexts.back() == PDF)
+ BidiContexts.pop_back();
+ }
+ // Open a PDI Context.
+ else if (CodePoint == RLI || CodePoint == LRI || CodePoint == FSI)
+ BidiContexts.push_back(PDI);
+ // Close a PDI Context.
+ else if (CodePoint == PDI) {
+ auto R = std::find(BidiContexts.rbegin(), BidiContexts.rend(), PDI);
+ if (R != BidiContexts.rend())
+ BidiContexts.resize(BidiContexts.rend() - R - 1);
+ }
+ // Line break or equivalent
+ else if (CodePoint == PS)
+ BidiContexts.clear();
+ }
+ return !BidiContexts.empty();
+}
+
+class MisleadingBidirectionalCheck::MisleadingBidirectionalHandler
+ : public CommentHandler {
+public:
+ MisleadingBidirectionalHandler(MisleadingBidirectionalCheck &Check,
+ llvm::Optional<std::string> User)
+ : Check(Check) {}
+
+ bool HandleComment(Preprocessor &PP, SourceRange Range) override {
+ // FIXME: check that we are in a /* */ comment
+ StringRef Text =
+ Lexer::getSourceText(CharSourceRange::getCharRange(Range),
+ PP.getSourceManager(), PP.getLangOpts());
+
+ if (containsMisleadingBidi(Text, true))
+ Check.diag(
+ Range.getBegin(),
+ "comment contains misleading bidirectional Unicode characters");
+ return false;
+ }
+
+private:
+ MisleadingBidirectionalCheck &Check;
+};
+
+MisleadingBidirectionalCheck::MisleadingBidirectionalCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ Handler(std::make_unique<MisleadingBidirectionalHandler>(
+ *this, Context->getOptions().User)) {}
+
+MisleadingBidirectionalCheck::~MisleadingBidirectionalCheck() = default;
+
+void MisleadingBidirectionalCheck::registerPPCallbacks(
+ const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+ PP->addCommentHandler(Handler.get());
+}
+
+void MisleadingBidirectionalCheck::check(
+ const ast_matchers::MatchFinder::MatchResult &Result) {
+ if (const auto *SL = Result.Nodes.getNodeAs<StringLiteral>("strlit")) {
+ StringRef Literal = SL->getBytes();
+ if (containsMisleadingBidi(Literal, false))
+ diag(SL->getBeginLoc(), "string literal contains misleading "
+ "bidirectional Unicode characters");
+ }
+}
+
+void MisleadingBidirectionalCheck::registerMatchers(
+ ast_matchers::MatchFinder *Finder) {
+ Finder->addMatcher(ast_matchers::stringLiteral().bind("strlit"), this);
+}
diff --git a/clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h b/clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
new file mode 100644
index 0000000000000..18e7060197a5d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
@@ -0,0 +1,38 @@
+//===--- MisleadingBidirectionalCheck.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_MISC_MISLEADINGBIDIRECTIONALCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+class MisleadingBidirectionalCheck : public ClangTidyCheck {
+public:
+ MisleadingBidirectionalCheck(StringRef Name, ClangTidyContext *Context);
+ ~MisleadingBidirectionalCheck();
+
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override;
+
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ class MisleadingBidirectionalHandler;
+ std::unique_ptr<MisleadingBidirectionalHandler> Handler;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 769eb2ba08038..098edd90b725f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,6 +127,10 @@ New checks
Reports identifiers whose names are too short. Currently checks local
variables and function parameters only.
+- New :doc:`misc-misleading-bidirectional <clang-tidy/checks/misc-misleading-bidirectional>` check.
+
+ Inspects string literal and comments for unterminated bidirectional Unicode
+ characters.
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 8d0a568cff881..74c5dd1217046 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,7 +212,8 @@ Clang-Tidy Checks
`llvmlibc-implementation-in-namespace <llvmlibc-implementation-in-namespace.html>`_,
`llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, "Yes"
`misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
- `misc-misleading-identifier <misc-misleading-identifier.html>`_,
+ `misc-misleading-bidirectional <misc-misleading-bidirectional.html>`_,
+ `misc-misleading-identifier <misc-mileading-identifier.html>`_,
`misc-misplaced-const <misc-misplaced-const.html>`_,
`misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
`misc-no-recursion <misc-no-recursion.html>`_,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
new file mode 100644
index 0000000000000..16ffc97d56ab4
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - misc-misleading-bidirectional
+
+misc-misleading-bidirectional
+=============================
+
+Warn about unterminated bidirectional unicode sequence, detecting potential attack
+as described in the `Trojan Source <https://www.trojansource.codes>`_ attack.
+
+Example:
+
+.. code-block:: c++
+
+ #include <iostream>
+
+ int main() {
+ bool isAdmin = false;
+ /* } if (isAdmin) begin admins only */
+ std::cout << "You are an admin.\n";
+ /* end admins only { */
+ return 0;
+ }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-misleading-bidirectional.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-misleading-bidirectional.cpp
new file mode 100644
index 0000000000000..5028bab297177
Binary files /dev/null and b/clang-tools-extra/test/clang-tidy/checkers/misc-misleading-bidirectional.cpp
diff er
More information about the cfe-commits
mailing list