Skip to content

Commit 4294d29

Browse files
simone-sanfratellolmamminolirantal
authored
feat: detect-bidi-characters rule (#95)
* Resolved conflicts in README * Embedded anti-trojan-source and removed from dependencies * Expanded README with an example * Changed onCodePath with Program * Improved code style as suggested in review * Added rough location estimation in the error report * feat: implement exact location for each bidi char * Renamed to detect-bidi-characters and fixed first line offset in comments * Added JSDoc in detectBidiCharacters * Fixed MD034 - Bare URL used * Apply suggestions from code review Co-authored-by: Liran Tal <[email protected]> Co-authored-by: Luciano Mammino <[email protected]> Co-authored-by: Liran Tal <[email protected]>
1 parent e060739 commit 4294d29

File tree

5 files changed

+235
-6
lines changed

5 files changed

+235
-6
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ npm test
5454

5555
| Name                                  | Description | ⚠️ |
5656
| :------------------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------- | :-- |
57+
| [detect-bidi-characters](docs/rules/detect-bidi-characters.md) | Detects trojan source attacks that employ unicode bidi attacks to inject malicious code. ||
5758
| [detect-buffer-noassert](docs/rules/detect-buffer-noassert.md) | Detects calls to "buffer" with "noAssert" flag set. ||
5859
| [detect-child-process](docs/rules/detect-child-process.md) | Detects instances of "child_process" & non-literal "exec()" calls. ||
5960
| [detect-disable-mustache-escape](docs/rules/detect-disable-mustache-escape.md) | Detects "object.escapeMarkup = false", which can be used with some template engines to disable escaping of HTML entities. ||

docs/rules/detect-bidi-characters.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Detects trojan source attacks that employ unicode bidi attacks to inject malicious code (`security/detect-bidi-characters`)
2+
3+
⚠️ This rule _warns_ in the ✅ `recommended` config.
4+
5+
<!-- end auto-generated rule header -->
6+
7+
Detects cases of [trojan source attacks](https://trojansource.codes) that employ unicode bidi attacks to inject malicious code
8+
9+
## Why is Trojan Source important?
10+
11+
The following publication on the topic of unicode characters attacks, dubbed [Trojan Source: Invisible Vulnerabilities](https://trojansource.codes/trojan-source.pdf), has caused a lot of concern from potential supply chain attacks where adversaries are able to inject malicious code into the source code of a project, slipping by unseen in the code review process.
12+
13+
### An example
14+
15+
As an example, take the following code where `RLO`, `LRI`, `PDI`, `IRI` are placeholders to visualise the respective dangerous unicode characters:
16+
17+
```js
18+
#!/usr/bin/env node
19+
20+
var accessLevel = "user";
21+
22+
if (accessLevel != "userRLO LRI// Check if adminPDI IRI") {
23+
console.log("You are an admin.");
24+
}
25+
```
26+
27+
The code above, will be rendered by a text editor as follows:
28+
29+
```js
30+
#!/usr/bin/env node
31+
32+
var accessLevel = "user";
33+
34+
if (accessLevel != "user") {
35+
// Check if admin
36+
console.log("You are an admin.");
37+
}
38+
```
39+
40+
By looking at the rendered code above, a user reviewing this code might not notice the injected malicious unicode characters which are actually changing the semantic and the behaviour of the actual code.
41+
42+
### More information
43+
44+
For more information on the topic, you're welcome to read on the official website [trojansource.codes](https://trojansource.codes/) and the following [source code repository](https://github.com/nickboucher/trojan-source/) which contains the source code of the publication.
45+
46+
### References
47+
48+
- <https://certitude.consulting/blog/en/invisible-backdoor/>
49+
- <https://github.com/lirantal/anti-trojan-source/>
50+
- <https://github.com/lirantal/eslint-plugin-anti-trojan-source>

index.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ module.exports = {
1818
'detect-child-process': require('./rules/detect-child-process'),
1919
'detect-disable-mustache-escape': require('./rules/detect-disable-mustache-escape'),
2020
'detect-object-injection': require('./rules/detect-object-injection'),
21-
'detect-new-buffer': require('./rules/detect-new-buffer')
21+
'detect-new-buffer': require('./rules/detect-new-buffer'),
22+
'detect-bidi-characters': require('./rules/detect-bidi-characters'),
2223
},
2324
rulesConfig: {
2425
'detect-unsafe-regex': 0,
@@ -33,7 +34,8 @@ module.exports = {
3334
'detect-child-process': 0,
3435
'detect-disable-mustache-escape': 0,
3536
'detect-object-injection': 0,
36-
'detect-new-buffer': 0
37+
'detect-new-buffer': 0,
38+
'detect-bidi-characters': 0,
3739
},
3840
configs: {
3941
recommended: {
@@ -51,8 +53,9 @@ module.exports = {
5153
'security/detect-object-injection': 'warn',
5254
'security/detect-possible-timing-attacks': 'warn',
5355
'security/detect-pseudoRandomBytes': 'warn',
54-
'security/detect-unsafe-regex': 'warn'
55-
}
56-
}
57-
}
56+
'security/detect-unsafe-regex': 'warn',
57+
'security/detect-bidi-characters': 'warn',
58+
},
59+
},
60+
},
5861
};

rules/detect-bidi-characters.js

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/**
2+
* Detect trojan source attacks that employ unicode bidi attacks to inject malicious code
3+
* @author Luciamo Mammino
4+
* @author Simone Sanfratello
5+
* @author Liran Tal
6+
*/
7+
8+
'use strict';
9+
10+
const dangerousBidiCharsRegexp = /[\u061C\u200E\u200F\u202A\u202B\u202C\u202D\u202E\u2066\u2067\u2068\u2069]/gu;
11+
12+
/**
13+
* Detects all the dangerous bidi characters in a given source text
14+
*
15+
* @param {object} options - Options
16+
* @param {string} options.sourceText - The source text to search for dangerous bidi characters
17+
* @param {number} options.firstLineOffset - The offset of the first line in the source text
18+
* @returns {Array<{line: number, column: number}>} - An array of reports, each report is an
19+
* object with the line and column of the dangerous character
20+
*/
21+
function detectBidiCharacters({ sourceText, firstLineOffset }) {
22+
const sourceTextToSearch = sourceText.toString();
23+
24+
const lines = sourceTextToSearch.split(/\r?\n/);
25+
26+
return lines.reduce((reports, line, lineIndex) => {
27+
let match;
28+
let offset = lineIndex == 0 ? firstLineOffset : 0;
29+
30+
while ((match = dangerousBidiCharsRegexp.exec(line)) !== null) {
31+
reports.push({ line: lineIndex, column: offset + match.index });
32+
}
33+
34+
return reports;
35+
}, []);
36+
}
37+
38+
function report({ context, node, tokens, message, firstLineOffset }) {
39+
if (!tokens || !Array.isArray(tokens)) {
40+
return;
41+
}
42+
tokens.forEach((token) => {
43+
const reports = detectBidiCharacters({ sourceText: token.value, firstLineOffset: token.loc.start.column + firstLineOffset });
44+
45+
reports.forEach((report) => {
46+
context.report({
47+
node: node,
48+
data: {
49+
text: token.value,
50+
},
51+
loc: {
52+
start: {
53+
line: token.loc.start.line + report.line,
54+
column: report.column,
55+
},
56+
end: {
57+
line: token.loc.start.line + report.line,
58+
column: report.column + 1,
59+
},
60+
},
61+
message,
62+
});
63+
});
64+
});
65+
}
66+
67+
//------------------------------------------------------------------------------
68+
// Rule Definition
69+
//------------------------------------------------------------------------------
70+
71+
module.exports = {
72+
meta: {
73+
type: 'error',
74+
docs: {
75+
description: 'Detects trojan source attacks that employ unicode bidi attacks to inject malicious code.',
76+
category: 'Possible Security Vulnerability',
77+
recommended: true,
78+
url: 'https://github.com/eslint-community/eslint-plugin-security/blob/main/docs/rules/detect-bidi-characters.md',
79+
},
80+
},
81+
create: function (context) {
82+
return {
83+
Program: function (node) {
84+
report({
85+
context,
86+
node,
87+
tokens: node.tokens,
88+
firstLineOffset: 0,
89+
message: "Detected potential trojan source attack with unicode bidi introduced in this code: '{{text}}'.",
90+
});
91+
report({
92+
context,
93+
node,
94+
tokens: node.comments,
95+
firstLineOffset: 2,
96+
message: "Detected potential trojan source attack with unicode bidi introduced in this comment: '{{text}}'.",
97+
});
98+
},
99+
};
100+
},
101+
};

test/detect-bidi-characters.js

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
'use strict';
2+
3+
const RuleTester = require('eslint').RuleTester;
4+
const tester = new RuleTester();
5+
6+
const ruleName = 'detect-bidi-characters';
7+
const Rule = require(`../rules/${ruleName}`);
8+
9+
tester.run(ruleName, Rule, {
10+
valid: [
11+
{
12+
code: `
13+
var accessLevel = "user";
14+
if (accessLevel != "user") { // Check if admin
15+
console.log("You are an admin.");
16+
}
17+
`,
18+
},
19+
],
20+
invalid: [
21+
{
22+
code: `
23+
var accessLevel = "user";
24+
if (accessLevel != "user‮ ⁦// Check if admin⁩ ⁦") {
25+
console.log("You are an admin.");
26+
}
27+
`,
28+
errors: [
29+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this code/i, line: 3, endLine: 3, column: 31, endColumn: 32 },
30+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this code/i, line: 3, endLine: 3, column: 33, endColumn: 34 },
31+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this code/i, line: 3, endLine: 3, column: 51, endColumn: 52 },
32+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this code/i, line: 3, endLine: 3, column: 53, endColumn: 54 },
33+
],
34+
},
35+
],
36+
});
37+
38+
tester.run(`${ruleName} in comment-line`, Rule, {
39+
valid: [
40+
{
41+
code: `
42+
var isAdmin = false;
43+
/* begin admins only */ if (isAdmin) {
44+
console.log("You are an admin.");
45+
/* end admins only */ }
46+
`,
47+
},
48+
],
49+
invalid: [
50+
{
51+
code: `
52+
var isAdmin = false;
53+
/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */
54+
console.log("You are an admin.");
55+
/* end admins only ‮
56+
⁦*/
57+
/* end admins only ‮
58+
{ ⁦*/
59+
`,
60+
errors: [
61+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 3, endLine: 3, column: 9, endColumn: 10 },
62+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 3, endLine: 3, column: 13, endColumn: 14 },
63+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 3, endLine: 3, column: 26, endColumn: 27 },
64+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 3, endLine: 3, column: 28, endColumn: 29 },
65+
66+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 5, endLine: 5, column: 26, endColumn: 27 },
67+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 6, endLine: 6, column: 1, endColumn: 2 },
68+
69+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 7, endLine: 7, column: 26, endColumn: 27 },
70+
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 8, endLine: 8, column: 4, endColumn: 5 },
71+
],
72+
},
73+
],
74+
});

0 commit comments

Comments
 (0)