Skip to content

Commit 35cca45

Browse files
author
serge-sans-paille
committed
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
1 parent e3275cf commit 35cca45

File tree

8 files changed

+208
-1
lines changed

8 files changed

+208
-1
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
66
add_clang_library(clangTidyMiscModule
77
DefinitionsInHeadersCheck.cpp
88
MiscTidyModule.cpp
9+
MisleadingBidirectional.cpp
910
MisleadingIdentifier.cpp
1011
MisplacedConstCheck.cpp
1112
NewDeleteOverloadsCheck.cpp

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "../ClangTidyModule.h"
1111
#include "../ClangTidyModuleRegistry.h"
1212
#include "DefinitionsInHeadersCheck.h"
13+
#include "MisleadingBidirectional.h"
1314
#include "MisleadingIdentifier.h"
1415
#include "MisplacedConstCheck.h"
1516
#include "NewDeleteOverloadsCheck.h"
@@ -34,6 +35,8 @@ class MiscModule : public ClangTidyModule {
3435
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
3536
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
3637
"misc-definitions-in-headers");
38+
CheckFactories.registerCheck<MisleadingBidirectionalCheck>(
39+
"misc-misleading-bidirectional");
3740
CheckFactories.registerCheck<MisleadingIdentifierCheck>(
3841
"misc-misleading-identifier");
3942
CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
//===--- MisleadingBidirectional.cpp - clang-tidy -------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "MisleadingBidirectional.h"
10+
11+
#include "clang/Frontend/CompilerInstance.h"
12+
#include "clang/Lex/Preprocessor.h"
13+
#include "llvm/Support/ConvertUTF.h"
14+
15+
using namespace clang;
16+
using namespace clang::tidy::misc;
17+
18+
static bool containsMisleadingBidi(StringRef Buffer,
19+
bool HonorLineBreaks = true) {
20+
const char *CurPtr = Buffer.begin();
21+
22+
enum BidiChar {
23+
PS = 0x2029,
24+
RLO = 0x202E,
25+
RLE = 0x202B,
26+
LRO = 0x202D,
27+
LRE = 0x202A,
28+
PDF = 0x202C,
29+
RLI = 0x2067,
30+
LRI = 0x2066,
31+
FSI = 0x2068,
32+
PDI = 0x2069
33+
};
34+
35+
SmallVector<BidiChar> BidiContexts;
36+
37+
// Scan each character while maintaining a stack of opened bidi context.
38+
// RLO/RLE/LRO/LRE all are closed by PDF while RLI LRI and FSI are closed by
39+
// PDI. New lines reset the context count. Extra PDF / PDI are ignored.
40+
//
41+
// Warn if we end up with an unclosed context.
42+
while (CurPtr < Buffer.end()) {
43+
unsigned char C = *CurPtr;
44+
if (isASCII(C)) {
45+
++CurPtr;
46+
bool IsParagrapSep =
47+
(C == 0xA || C == 0xD || (0x1C <= C && C <= 0x1E) || C == 0x85);
48+
bool IsSegmentSep = (C == 0x9 || C == 0xB || C == 0x1F);
49+
if (IsParagrapSep || IsSegmentSep)
50+
BidiContexts.clear();
51+
continue;
52+
}
53+
llvm::UTF32 CodePoint;
54+
llvm::ConversionResult Result = llvm::convertUTF8Sequence(
55+
(const llvm::UTF8 **)&CurPtr, (const llvm::UTF8 *)Buffer.end(),
56+
&CodePoint, llvm::strictConversion);
57+
58+
// If conversion fails, utf-8 is designed so that we can just try next char.
59+
if (Result != llvm::conversionOK) {
60+
++CurPtr;
61+
continue;
62+
}
63+
64+
// Open a PDF context.
65+
if (CodePoint == RLO || CodePoint == RLE || CodePoint == LRO ||
66+
CodePoint == LRE)
67+
BidiContexts.push_back(PDF);
68+
// Close PDF Context.
69+
else if (CodePoint == PDF) {
70+
if (!BidiContexts.empty() && BidiContexts.back() == PDF)
71+
BidiContexts.pop_back();
72+
}
73+
// Open a PDI Context.
74+
else if (CodePoint == RLI || CodePoint == LRI || CodePoint == FSI)
75+
BidiContexts.push_back(PDI);
76+
// Close a PDI Context.
77+
else if (CodePoint == PDI) {
78+
auto R = std::find(BidiContexts.rbegin(), BidiContexts.rend(), PDI);
79+
if (R != BidiContexts.rend())
80+
BidiContexts.resize(BidiContexts.rend() - R - 1);
81+
}
82+
// Line break or equivalent
83+
else if (CodePoint == PS)
84+
BidiContexts.clear();
85+
}
86+
return !BidiContexts.empty();
87+
}
88+
89+
class MisleadingBidirectionalCheck::MisleadingBidirectionalHandler
90+
: public CommentHandler {
91+
public:
92+
MisleadingBidirectionalHandler(MisleadingBidirectionalCheck &Check,
93+
llvm::Optional<std::string> User)
94+
: Check(Check) {}
95+
96+
bool HandleComment(Preprocessor &PP, SourceRange Range) override {
97+
// FIXME: check that we are in a /* */ comment
98+
StringRef Text =
99+
Lexer::getSourceText(CharSourceRange::getCharRange(Range),
100+
PP.getSourceManager(), PP.getLangOpts());
101+
102+
if (containsMisleadingBidi(Text, true))
103+
Check.diag(
104+
Range.getBegin(),
105+
"comment contains misleading bidirectional Unicode characters");
106+
return false;
107+
}
108+
109+
private:
110+
MisleadingBidirectionalCheck &Check;
111+
};
112+
113+
MisleadingBidirectionalCheck::MisleadingBidirectionalCheck(
114+
StringRef Name, ClangTidyContext *Context)
115+
: ClangTidyCheck(Name, Context),
116+
Handler(std::make_unique<MisleadingBidirectionalHandler>(
117+
*this, Context->getOptions().User)) {}
118+
119+
MisleadingBidirectionalCheck::~MisleadingBidirectionalCheck() = default;
120+
121+
void MisleadingBidirectionalCheck::registerPPCallbacks(
122+
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
123+
PP->addCommentHandler(Handler.get());
124+
}
125+
126+
void MisleadingBidirectionalCheck::check(
127+
const ast_matchers::MatchFinder::MatchResult &Result) {
128+
if (const auto *SL = Result.Nodes.getNodeAs<StringLiteral>("strlit")) {
129+
StringRef Literal = SL->getBytes();
130+
if (containsMisleadingBidi(Literal, false))
131+
diag(SL->getBeginLoc(), "string literal contains misleading "
132+
"bidirectional Unicode characters");
133+
}
134+
}
135+
136+
void MisleadingBidirectionalCheck::registerMatchers(
137+
ast_matchers::MatchFinder *Finder) {
138+
Finder->addMatcher(ast_matchers::stringLiteral().bind("strlit"), this);
139+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//===--- MisleadingBidirectionalCheck.h - clang-tidy ------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang {
15+
namespace tidy {
16+
namespace misc {
17+
18+
class MisleadingBidirectionalCheck : public ClangTidyCheck {
19+
public:
20+
MisleadingBidirectionalCheck(StringRef Name, ClangTidyContext *Context);
21+
~MisleadingBidirectionalCheck();
22+
23+
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
24+
Preprocessor *ModuleExpanderPP) override;
25+
26+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
27+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
28+
29+
private:
30+
class MisleadingBidirectionalHandler;
31+
std::unique_ptr<MisleadingBidirectionalHandler> Handler;
32+
};
33+
34+
} // namespace misc
35+
} // namespace tidy
36+
} // namespace clang
37+
38+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ New checks
127127
Reports identifiers whose names are too short. Currently checks local
128128
variables and function parameters only.
129129

130+
- New :doc:`misc-misleading-bidirectional <clang-tidy/checks/misc-misleading-bidirectional>` check.
131+
132+
Inspects string literal and comments for unterminated bidirectional Unicode
133+
characters.
130134

131135
New check aliases
132136
^^^^^^^^^^^^^^^^^

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,8 @@ Clang-Tidy Checks
212212
`llvmlibc-implementation-in-namespace <llvmlibc-implementation-in-namespace.html>`_,
213213
`llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, "Yes"
214214
`misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
215-
`misc-misleading-identifier <misc-misleading-identifier.html>`_,
215+
`misc-misleading-bidirectional <misc-misleading-bidirectional.html>`_,
216+
`misc-misleading-identifier <misc-mileading-identifier.html>`_,
216217
`misc-misplaced-const <misc-misplaced-const.html>`_,
217218
`misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
218219
`misc-no-recursion <misc-no-recursion.html>`_,
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
.. title:: clang-tidy - misc-misleading-bidirectional
2+
3+
misc-misleading-bidirectional
4+
=============================
5+
6+
Warn about unterminated bidirectional unicode sequence, detecting potential attack
7+
as described in the `Trojan Source <https://www.trojansource.codes>`_ attack.
8+
9+
Example:
10+
11+
.. code-block:: c++
12+
13+
#include <iostream>
14+
15+
int main() {
16+
bool isAdmin = false;
17+
/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */
18+
std::cout << "You are an admin.\n";
19+
/* end admins only ‮ { ⁦*/
20+
return 0;
21+
}
Binary file not shown.

0 commit comments

Comments
 (0)