Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -6,6 +6,7 @@ add_clang_library(clangTidyMiscModule DefinitionsInHeadersCheck.cpp MiscTidyModule.cpp + MisleadingBidirectional.cpp MisplacedConstCheck.cpp NewDeleteOverloadsCheck.cpp NoRecursionCheck.cpp Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ 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 "MisplacedConstCheck.h" #include "NewDeleteOverloadsCheck.h" #include "NoRecursionCheck.h" @@ -33,6 +34,8 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "misc-definitions-in-headers"); + CheckFactories.registerCheck( + "misc-misleading-bidirectional"); CheckFactories.registerCheck("misc-misplaced-const"); CheckFactories.registerCheck( "misc-new-delete-overloads"); Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h =================================================================== --- /dev/null +++ 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 Handler; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp @@ -0,0 +1,131 @@ +//===--- 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; + +static bool containsMisleadingBidi(StringRef Buffer, + bool HonorLineBreaks = true) { + const char *CurPtr = Buffer.begin(); + unsigned EmbeddingOverride = 0, Isolate = 0; + unsigned i = 0; + + enum { + LS = 0x2028, + PS = 0x2029, + RLO = 0x202E, + RLE = 0x202B, + LRO = 0x202D, + LRE = 0x202A, + PDF = 0x202C, + RLI = 0x2067, + LRI = 0x2066, + FSI = 0x2068, + PDI = 0x2069 + }; + + // Scan each character while maintaining a count 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()) { + ++i; + unsigned char C = *CurPtr; + if (isASCII(C)) { + ++CurPtr; + // Line break: https://www.unicode.org/reports/tr14/tr14-32.html + if (C == '\n' || C == '\r' || C == '\f' || C == '\v' || + C == 0x85 /*next line*/) + EmbeddingOverride = Isolate = 0; + 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; + } + + if (CodePoint == RLO || CodePoint == RLE || CodePoint == LRO || + CodePoint == LRE) + EmbeddingOverride += 1; + else if (CodePoint == PDF) + EmbeddingOverride = std::min(EmbeddingOverride - 1, EmbeddingOverride); + else if (CodePoint == RLI || CodePoint == LRI || CodePoint == FSI) + Isolate += 1; + else if (CodePoint == PDI) + Isolate = std::min(Isolate - 1, Isolate); + // Line break: https://www.unicode.org/reports/tr14/tr14-32.html + else if (CodePoint == LS || CodePoint == PS) + EmbeddingOverride = Isolate = 0; + } + return EmbeddingOverride != 0 || Isolate != 0; +} + +class clang::tidy::misc::MisleadingBidirectionalCheck:: + MisleadingBidirectionalHandler : public CommentHandler { +public: + MisleadingBidirectionalHandler(MisleadingBidirectionalCheck &Check, + llvm::Optional 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; +}; + +clang::tidy::misc::MisleadingBidirectionalCheck::MisleadingBidirectionalCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Handler(std::make_unique( + *this, Context->getOptions().User)) {} + +clang::tidy::misc::MisleadingBidirectionalCheck:: + ~MisleadingBidirectionalCheck() = default; + +void clang::tidy::misc::MisleadingBidirectionalCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + PP->addCommentHandler(Handler.get()); +} + +void clang::tidy::misc::MisleadingBidirectionalCheck::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + if (const auto *SL = Result.Nodes.getNodeAs("strlit")) { + StringRef Literal = SL->getBytes(); + if (containsMisleadingBidi(Literal, false)) + diag(SL->getBeginLoc(), "string literal contains misleading " + "bidirectional Unicode characters"); + } +} + +void clang::tidy::misc::MisleadingBidirectionalCheck::registerMatchers( + ast_matchers::MatchFinder *Finder) { + Finder->addMatcher(ast_matchers::stringLiteral().bind("strlit"), this); +} Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -100,6 +100,11 @@ Finds cases where code could use ``data()`` rather than the address of the element at index 0 in a container. +- New :doc:`misc-misleading-bidirectional ` check. + + Inspect string literal and comments for unterminated bidirectional Unicode + characters. + New check aliases ^^^^^^^^^^^^^^^^^ Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -211,6 +211,7 @@ `llvmlibc-implementation-in-namespace `_, `llvmlibc-restrict-system-libc-headers `_, "Yes" `misc-definitions-in-headers `_, "Yes" + `misc-misleading-bidirectional `_, `misc-misplaced-const `_, `misc-new-delete-overloads `_, `misc-no-recursion `_, Index: clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst =================================================================== --- /dev/null +++ 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 `_ attack. + +Example: + +.. code-block:: c++ + + #include + + int main() { + bool isAdmin = false; + /*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */ + std::cout << "You are an admin.\n"; + /* end admins only ‮ { ⁦*/ + return 0; + } Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py =================================================================== --- clang-tools-extra/test/clang-tidy/check_clang_tidy.py +++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py @@ -34,7 +34,7 @@ def write_file(file_name, text): - with open(file_name, 'w') as f: + with open(file_name, 'w', encoding='utf-8') as f: f.write(text) f.truncate() @@ -82,7 +82,7 @@ if resource_dir is not None: clang_extra_args.append('-resource-dir=%s' % resource_dir) - with open(input_file_name, 'r') as input_file: + with open(input_file_name, 'r', encoding="utf-8") as input_file: input_text = input_file.read() check_fixes_prefixes = [] Index: clang-tools-extra/test/clang-tidy/checkers/misc-misleading-bidirectional.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/misc-misleading-bidirectional.cpp @@ -0,0 +1,31 @@ +// RUN: %check_clang_tidy %s misc-misleading-bidirectional %t + +void func(void) { + int admin = 0; + /*‮ }⁦if(admin)⁩ ⁦ begin*/ + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comment contains misleading bidirectional Unicode characters [misc-misleading-bidirectional] + const char msg[] = "‮⁦if(admin)⁩ ⁦tes"; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: string literal contains misleading bidirectional Unicode characters [misc-misleading-bidirectional] +} + +void all_fine(void) { + char valid[] = "some‮valid‬sequence"; + /* EOL ends bidi‮ sequence + * end it's fine to do so. + * EOL ends ⁧isolate too + */ +} + +int invalid_utf_8(void) { + bool isAdmin = false; + + // the comment below contains an invalid utf8 character, but should still be + // processed. + + // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: comment contains misleading bidirectional Unicode characters [misc-misleading-bidirectional] + /*€‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */ + return 1; + /* end admins only ‮ { ⁦*/ + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: comment contains misleading bidirectional Unicode characters [misc-misleading-bidirectional] + return 0; +}