Skip to content

Commit c1ed6b1

Browse files
committed
Fix LinkifyIt match detection
1 parent d723645 commit c1ed6b1

7 files changed

Lines changed: 66 additions & 0 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Calls to `LinkifyIt.match()` are no longer incorrectly identified as regular expression operations.

‎javascript/ql/lib/semmle/javascript/Regexp.qll‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,22 @@ private predicate isMatchObjectProperty(string name) {
986986
name in ["length", "index", "input", "groups"]
987987
}
988988

989+
/** Gets an API node representing a `LinkifyIt` instance. */
990+
private API::Node linkifyItInstance() {
991+
result = API::moduleImport("linkify-it").getMember("exports").getMember("LinkifyIt").getInstance()
992+
or
993+
result = API::moduleImport("linkify-it").getMember("LinkifyIt").getInstance()
994+
or
995+
result = API::moduleImport("linkify-it").getMember("exports").getMember("linkifyit").getReturn()
996+
or
997+
result = API::moduleImport("linkify-it").getMember("linkifyit").getReturn()
998+
or
999+
// Before version 6, the module export was the factory function.
1000+
result = API::moduleImport("linkify-it").getReturn()
1001+
or
1002+
result = linkifyItInstance().getMember(["add", "set", "tlds"]).getReturn()
1003+
}
1004+
9891005
/** Holds if `call` is a call to `match` whose result is used in a way that is incompatible with Match objects. */
9901006
overlay[global]
9911007
private predicate isUsedAsNonMatchObject(DataFlow::MethodCallNode call) {
@@ -1006,6 +1022,8 @@ private predicate isUsedAsNonMatchObject(DataFlow::MethodCallNode call) {
10061022
call.asExpr() = any(ExprStmt stmt).getExpr()
10071023
or
10081024
call = API::moduleImport("sinon").getMember("match").getACall()
1025+
or
1026+
call = linkifyItInstance().getMember("match").getACall()
10091027
)
10101028
}
10111029

‎javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
| linkify-it/tst-LinkifyIt.js:21:25:21:48 | ^https://www.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | linkify-it/tst-LinkifyIt.js:21:24:21:49 | "^https ... le.com" | here |
12
| tst-IncompleteHostnameRegExp.js:3:3:3:28 | ^http:\\/\\/test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:3:2:3:29 | /^http: ... le.com/ | here |
23
| tst-IncompleteHostnameRegExp.js:6:3:6:28 | ^http:\\/\\/test.example.net | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:6:2:6:29 | /^http: ... le.net/ | here |
34
| tst-IncompleteHostnameRegExp.js:7:3:7:42 | ^http:\\/\\/test.(example-a\|example-b).com | This regular expression has an unescaped '.' before '(example-a\|example-b).com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:7:2:7:43 | /^http: ... b).com/ | here |
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "module",
3+
"dependencies": {
4+
"linkify-it": "6.1.0"
5+
}
6+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { LinkifyIt, linkifyit } from "linkify-it";
2+
import { LinkifyIt as OtherLinkifyIt } from "other-linkify-it";
3+
4+
const scanner = new LinkifyIt({ fuzzyLink: false, fuzzyEmail: false })
5+
.add("ftp:", null)
6+
.add("mailto:", null)
7+
.add("//", null);
8+
const text =
9+
"😀 *literal* (https://www.youtube.com/watch?v=tax4e4hBBZc), then https://store.steampowered.com/app/457140/.";
10+
const matches = scanner.match(text);
11+
if (matches) {
12+
console.log(matches.map((match) => match.raw));
13+
}
14+
15+
if (new LinkifyIt().match("https://www.example.com")) {}
16+
if (new LinkifyIt().set({ fuzzyLink: false }).match("https://www.example.com")) {}
17+
if (new LinkifyIt().tlds("onion", true).match("https://www.example.com")) {}
18+
if (linkifyit().match("https://www.example.com")) {}
19+
20+
const otherScanner = new OtherLinkifyIt().add("ftp:", null);
21+
if (otherScanner.match("^https://www.example.com")) {} // $ Alert
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
const { LinkifyIt } = require("linkify-it");
2+
const legacyLinkifyIt = require("linkify-it");
3+
4+
const scanner = new LinkifyIt().add("ftp:", null).set({ fuzzyLink: false });
5+
const text = "https://a.b.com";
6+
console.log(scanner.match(text));
7+
console.log(legacyLinkifyIt().match(text));

‎javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjectionGood.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,12 @@ app.get('/findKey', function(req, res) {
99
var safeKey = _.escapeRegExp(key);
1010
var re = new RegExp("\\b" + safeKey + "=(.*)\n");
1111
});
12+
13+
var { LinkifyIt } = require("linkify-it");
14+
15+
app.get('/findLinks', function(req, res) {
16+
var text = req.param("text");
17+
var scanner = new LinkifyIt().set({ fuzzyLink: false });
18+
var matches = scanner.match(text);
19+
res.json(matches);
20+
});

0 commit comments

Comments
 (0)