Thoughts about using an eslint plugin for REDOS detection?

[davisjam]

This is a question for discussion, not an issue.

• I am working on a vuln-regex-detector module.
• I will be creating an eslint plugin based on this module in the next day or two. It will make one synchronous HTTP query for each regex it finds in the source code. Queries go to a server I host at Virginia Tech which has a database mapping regexes to pre-computed "is vulnerable?" answers. The server is running the server side of the code in this project.

Once it's ready, does anyone have concerns with adding my REDOS plugin to this project's eslint config?

[joshbruce]

Tagging @UziTech

[UziTech]

It would be really nice to get some sort of ReDoS test on travis. Could we make it so the server side code runs on travis instead of making http requests?

[davisjam]

@UziTech

Could we make it so the server side code runs on travis instead of making http requests?

Testing a regex is expensive (potentially 60 seconds+ per regex) so memoizing them is helpful. Then that memoized result has to live somewhere.

My impression (could be wrong) is that Travis isn't a good place for permanent storage.
Is there something wrong with HTTP requests?

[UziTech]

Is there something wrong with HTTP requests?

In my experience http requests from travis seem to be the least reliable part of travis and "one synchronous HTTP query for each regex" could be a lot of http requests for every pr.

Could we download the memoized results and check the regexes ourselves or send a bunch of regexes at once to limit the number of http requests?

[davisjam]

Could we download the memoized results and check the regexes ourselves?

100MB+, not a good option.

or send a bunch of regexes at once to limit the number of http requests?

I don't think you can batch nicely with an eslint plugin. The "is vulnerable?" function gets called for each regex node in the AST and doesn't have a bigger sense of the context.

If you want to set up a post-eslint "security scanner" that runs as part of npm run test or npm run lint, you could do batching that way.

Could be a lot of http requests for every PR

About 100 requests.

grep '/.*/' lib/marked.js | grep -v '\/\/' | wc -l
98

[UziTech]

I would rather it be separate from eslint so we could run tests, run redos tests, then run lint. I feel like that would make it easier to see what failed in travis.

[davisjam]

@UziTech The eslint plugin is now available here.

Here are the steps I took:

1. Clone marked to my laptop (several miles from my server and using wifi)
2. Add the eslint plugin and enable the rule
3. Time how long it takes to run npm run lint with and without the rule

Here's what I found:

• Without plugin, linting takes 2 seconds
• With plugin, linting takes 16 seconds

See Slack for additional notes.

[styfle]

@davisjam First of all, this is amazing work!

However, I agree with UziTech that this is less than ideal for every build to fire off HTTP requests. Especially for a "lint" test. Linting should be fast and it would be very odd to be working on code in a Plane without wifi and you can't run lint for formatting errors.

Can this be a separate build step? Why does it need to be an eslint plugin?

[UziTech]

I agree with @styfle. I have my IDE running eslint in the background and underlining code as I type, it would be pretty annoying to have to wait 16 seconds to get that visual cue as I'm typing.

[davisjam]

@styfle @UziTech

it would be very odd to be working on code in a Plane without wifi and you can't run lint for formatting errors.

If the server is inaccessible, the vuln-regex-detector module returns "INVALID" and the eslint plugin ignores that regex. So there wouldn't be linter errors. It would still be slow though.

Can this be a separate build step? Why does it need to be an eslint plugin?

There's no particular reason it has to be an eslint plugin, other than convenience.

I was exploring an eslint plugin for REDOS detection, along the lines of eslint-plugin-safe-regex and eslint-plugin-security.

eslint handles the jobs any such tool needs:

• identifying the files to scan
• giving you an AST to traverse

Identifying REDOS issues without false positives requires either (1) expensive local computation or (2) hitting a server. So running this at the same time as the other linting rules (e.g. for use in an IDE as @UziTech notes) is not ideal because of the performance hit.

Does anyone know of a good pattern for running this kind of check during the programming cycle?

What are your thoughts about adding it as part of npm run test, implemented as a separate scan for regexes on a designated set of files of interest (in this case, lib/marked.js)? This approach enables batching so it should be faster.

[styfle]

npm run test:regex would be good

[davisjam]

How about this?

"test:regex": "eslint --plugin vuln-regex-detector --rule '\"vuln-regex-detector/no-vuln-regex\": 2' lib/marked.js",

It's still slow because it's making synchronous requests for each regex. But since it's off the linter fast path I imagine that's OK now.

If acceptable, I'll open a PR.

[davisjam]

Time estimate: From an AWS micro instance, it takes 25 seconds to run npm run test:regex.

I'm working on a local cache of results so on dev machines the overhead would be minimal. On Travis it would still be in the "cost of 100 queries" range.