ajinabraham/njsscan

false positive for regex_injection_dos

dogmatic69 opened this issue · 1 comments

This started failing recently, not exactly sure when due to unrelated issues. Almost certainly in the last week or two. I'm using the docker image opensecurity/njsscan:latest in github actions so always pulls fresh with no cache.

For context the fooo object is injected as middleware and has a method named search. With in that method there is regex on "user" input so initially I thought the line matching was wrong.

Unable to find the cause / fix, I decided to try change the function name and suddenly it no longer fails the scan.

In short, seems what has tripped it up is that the function being invoked is named search.

njsscan: v0.3.2 | Ajin Abraham | opensecurity.in
╒═════════════╤══════════════════════════════════════════════════════════════════════════════════════╕
│ RULE ID     │ regex_injection_dos                                                                  │
├─────────────┼──────────────────────────────────────────────────────────────────────────────────────┤
│ CWE         │ CWE-400: Uncontrolled Resource Consumption                                           │
├─────────────┼──────────────────────────────────────────────────────────────────────────────────────┤
│ OWASP-WEB   │ A1: Injection                                                                        │
├─────────────┼──────────────────────────────────────────────────────────────────────────────────────┤
│ DESCRIPTION │ User controlled data in RegExp() can make the application vulnerable to layer 7 DoS. │
├─────────────┼──────────────────────────────────────────────────────────────────────────────────────┤
│ SEVERITY    │ ERROR                                                                                │
├─────────────┼──────────────────────────────────────────────────────────────────────────────────────┤
│ FILES       │ ╒════════════════╤═══════════════════════════════════════════════════════════════╕   │
│             │ │ File           │ /src/controller.js                                            │   │
│             │ ├────────────────┼───────────────────────────────────────────────────────────────┤   │
│             │ │ Match Position │ 26 - 7                                                        │   │
│             │ ├────────────────┼───────────────────────────────────────────────────────────────┤   │
│             │ │ Line Number(s) │ 13: 21                                                        │   │
│             │ ├────────────────┼───────────────────────────────────────────────────────────────┤   │
│             │ │ Match String   │ const issues = await req.fooo.search({                        │   │
│             │ │                │       teams: req.config.fooo.teams,                           │   │
│             │ │                │       start,                                                  │   │
│             │ │                │       end,                                                    │   │
│             │ │                │       maxResults,                                             │   │
│             │ │                │       statuses: (statuses || '').split(',').filter((s) => s), │   │
│             │ │                │     });                                                       │   │
│             │ ╘════════════════╧═══════════════════════════════════════════════════════════════╛   │
╘═════════════╧══════════════════════════════════════════════════════════════════════════════════════╛

Perhaps downstream, if so feel free to point me where to look.

This is a false positive caused by

$STR.search(<... $REQ.$PARAM.$BAR ...>)

Due to limitations of semgrep, there is no feasible way to fix this. You could ignore the finding using suppression comments.