Skip to content

Commit

Permalink
feat: detect-bidi-characters rule (#95)
Browse files Browse the repository at this point in the history
* 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 <liran.tal@gmail.com>

Co-authored-by: Luciano Mammino <lucianomammino@gmail.com>
Co-authored-by: Liran Tal <liran.tal@gmail.com>
  • Loading branch information
3 people committed Jan 2, 2023
1 parent e060739 commit 4294d29
Show file tree
Hide file tree
Showing 5 changed files with 235 additions and 6 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ npm test

| Name                                  | Description | ⚠️ |
| :------------------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------- | :-- |
| [detect-bidi-characters](docs/rules/detect-bidi-characters.md) | Detects trojan source attacks that employ unicode bidi attacks to inject malicious code. ||
| [detect-buffer-noassert](docs/rules/detect-buffer-noassert.md) | Detects calls to "buffer" with "noAssert" flag set. ||
| [detect-child-process](docs/rules/detect-child-process.md) | Detects instances of "child_process" & non-literal "exec()" calls. ||
| [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. ||
Expand Down
50 changes: 50 additions & 0 deletions docs/rules/detect-bidi-characters.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Detects trojan source attacks that employ unicode bidi attacks to inject malicious code (`security/detect-bidi-characters`)

⚠️ This rule _warns_ in the ✅ `recommended` config.

<!-- end auto-generated rule header -->

Detects cases of [trojan source attacks](https://trojansource.codes) that employ unicode bidi attacks to inject malicious code

## Why is Trojan Source important?

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.

### An example

As an example, take the following code where `RLO`, `LRI`, `PDI`, `IRI` are placeholders to visualise the respective dangerous unicode characters:

```js
#!/usr/bin/env node

var accessLevel = "user";

if (accessLevel != "userRLO LRI// Check if adminPDI IRI") {
console.log("You are an admin.");
}
```

The code above, will be rendered by a text editor as follows:

```js
#!/usr/bin/env node

var accessLevel = "user";

if (accessLevel != "user") {
// Check if admin
console.log("You are an admin.");
}
```

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.

### More information

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.

### References

- <https://certitude.consulting/blog/en/invisible-backdoor/>
- <https://github.com/lirantal/anti-trojan-source/>
- <https://github.com/lirantal/eslint-plugin-anti-trojan-source>
15 changes: 9 additions & 6 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ module.exports = {
'detect-child-process': require('./rules/detect-child-process'),
'detect-disable-mustache-escape': require('./rules/detect-disable-mustache-escape'),
'detect-object-injection': require('./rules/detect-object-injection'),
'detect-new-buffer': require('./rules/detect-new-buffer')
'detect-new-buffer': require('./rules/detect-new-buffer'),
'detect-bidi-characters': require('./rules/detect-bidi-characters'),
},
rulesConfig: {
'detect-unsafe-regex': 0,
Expand All @@ -33,7 +34,8 @@ module.exports = {
'detect-child-process': 0,
'detect-disable-mustache-escape': 0,
'detect-object-injection': 0,
'detect-new-buffer': 0
'detect-new-buffer': 0,
'detect-bidi-characters': 0,
},
configs: {
recommended: {
Expand All @@ -51,8 +53,9 @@ module.exports = {
'security/detect-object-injection': 'warn',
'security/detect-possible-timing-attacks': 'warn',
'security/detect-pseudoRandomBytes': 'warn',
'security/detect-unsafe-regex': 'warn'
}
}
}
'security/detect-unsafe-regex': 'warn',
'security/detect-bidi-characters': 'warn',
},
},
},
};
101 changes: 101 additions & 0 deletions rules/detect-bidi-characters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* Detect trojan source attacks that employ unicode bidi attacks to inject malicious code
* @author Luciamo Mammino
* @author Simone Sanfratello
* @author Liran Tal
*/

'use strict';

const dangerousBidiCharsRegexp = /[\u061C\u200E\u200F\u202A\u202B\u202C\u202D\u202E\u2066\u2067\u2068\u2069]/gu;

/**
* Detects all the dangerous bidi characters in a given source text
*
* @param {object} options - Options
* @param {string} options.sourceText - The source text to search for dangerous bidi characters
* @param {number} options.firstLineOffset - The offset of the first line in the source text
* @returns {Array<{line: number, column: number}>} - An array of reports, each report is an
* object with the line and column of the dangerous character
*/
function detectBidiCharacters({ sourceText, firstLineOffset }) {
const sourceTextToSearch = sourceText.toString();

const lines = sourceTextToSearch.split(/\r?\n/);

return lines.reduce((reports, line, lineIndex) => {
let match;
let offset = lineIndex == 0 ? firstLineOffset : 0;

while ((match = dangerousBidiCharsRegexp.exec(line)) !== null) {
reports.push({ line: lineIndex, column: offset + match.index });
}

return reports;
}, []);
}

function report({ context, node, tokens, message, firstLineOffset }) {
if (!tokens || !Array.isArray(tokens)) {
return;
}
tokens.forEach((token) => {
const reports = detectBidiCharacters({ sourceText: token.value, firstLineOffset: token.loc.start.column + firstLineOffset });

reports.forEach((report) => {
context.report({
node: node,
data: {
text: token.value,
},
loc: {
start: {
line: token.loc.start.line + report.line,
column: report.column,
},
end: {
line: token.loc.start.line + report.line,
column: report.column + 1,
},
},
message,
});
});
});
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
type: 'error',
docs: {
description: 'Detects trojan source attacks that employ unicode bidi attacks to inject malicious code.',
category: 'Possible Security Vulnerability',
recommended: true,
url: 'https://github.com/eslint-community/eslint-plugin-security/blob/main/docs/rules/detect-bidi-characters.md',
},
},
create: function (context) {
return {
Program: function (node) {
report({
context,
node,
tokens: node.tokens,
firstLineOffset: 0,
message: "Detected potential trojan source attack with unicode bidi introduced in this code: '{{text}}'.",
});
report({
context,
node,
tokens: node.comments,
firstLineOffset: 2,
message: "Detected potential trojan source attack with unicode bidi introduced in this comment: '{{text}}'.",
});
},
};
},
};
74 changes: 74 additions & 0 deletions test/detect-bidi-characters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
'use strict';

const RuleTester = require('eslint').RuleTester;
const tester = new RuleTester();

const ruleName = 'detect-bidi-characters';
const Rule = require(`../rules/${ruleName}`);

tester.run(ruleName, Rule, {
valid: [
{
code: `
var accessLevel = "user";
if (accessLevel != "user") { // Check if admin
console.log("You are an admin.");
}
`,
},
],
invalid: [
{
code: `
var accessLevel = "user";
if (accessLevel != "user‮ ⁦// Check if admin⁩ ⁦") {
console.log("You are an admin.");
}
`,
errors: [
{ message: /Detected potential trojan source attack with unicode bidi introduced in this code/i, line: 3, endLine: 3, column: 31, endColumn: 32 },
{ message: /Detected potential trojan source attack with unicode bidi introduced in this code/i, line: 3, endLine: 3, column: 33, endColumn: 34 },
{ message: /Detected potential trojan source attack with unicode bidi introduced in this code/i, line: 3, endLine: 3, column: 51, endColumn: 52 },
{ message: /Detected potential trojan source attack with unicode bidi introduced in this code/i, line: 3, endLine: 3, column: 53, endColumn: 54 },
],
},
],
});

tester.run(`${ruleName} in comment-line`, Rule, {
valid: [
{
code: `
var isAdmin = false;
/* begin admins only */ if (isAdmin) {
console.log("You are an admin.");
/* end admins only */ }
`,
},
],
invalid: [
{
code: `
var isAdmin = false;
/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */
console.log("You are an admin.");
/* end admins only ‮
⁦*/
/* end admins only ‮
{ ⁦*/
`,
errors: [
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 3, endLine: 3, column: 9, endColumn: 10 },
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 3, endLine: 3, column: 13, endColumn: 14 },
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 3, endLine: 3, column: 26, endColumn: 27 },
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 3, endLine: 3, column: 28, endColumn: 29 },

{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 5, endLine: 5, column: 26, endColumn: 27 },
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 6, endLine: 6, column: 1, endColumn: 2 },

{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 7, endLine: 7, column: 26, endColumn: 27 },
{ message: /Detected potential trojan source attack with unicode bidi introduced in this comment/i, line: 8, endLine: 8, column: 4, endColumn: 5 },
],
},
],
});

0 comments on commit 4294d29

Please sign in to comment.