Skip to content

Conversation

erik-krogh
Copy link
Contributor

And adds a Fs module similar to the NodeJSLib::Path module.

Adds support for the sink in CVE-2018-1002204

@erik-krogh erik-krogh added the JS label May 14, 2020
@erik-krogh erik-krogh requested a review from a team as a code owner May 14, 2020 18:34
Copy link
Contributor

@esbena esbena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Lets update the change note to:

| Zip Slip (`js/zipslip`) | More results | This query now recognizes additional vulnerabilities. |

Super minor nit: the module name Fs is awkward to look at. Asger, will you give a 👍/👎 on that name?

@asgerf
Copy link
Contributor

asgerf commented May 15, 2020

the module name Fs is awkward to look at. Asger, will you give a 👍/👎 on that name?

I agree. Could you rename it to NodeJSLib::FS?

When it comes to capitalization I think the Dart style guide got it exactly right, and I personally try to stick with that. In particular, two-letter acronyms are fully capitalized, so you get UIHandler instead of the ugly UiHandler, and likewise, FS instead of Fs.

@erik-krogh
Copy link
Contributor Author

erik-krogh commented May 16, 2020

@semmle-qlci semmle-qlci merged commit 8d41ce1 into github:master May 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants