Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vuln #571

Closed
wadahkode opened this issue Jan 22, 2021 · 3 comments
Closed

vuln #571

wadahkode opened this issue Jan 22, 2021 · 3 comments

Comments

@wadahkode
Copy link

how to fix?

let ejs = require('ejs') ejs.render('./views/test.ejs',{ filename:'/etc/passwd\nfinally { this.global.process.mainModule.require(\'child_process\').execSync(\'touch EJS_HACKED\') }', compileDebug: true, message: 'test', client: true })

@mde
Copy link
Owner

mde commented Jan 22, 2021

This is not a problem with EJS.

EJS literally evaluates the JavaScript you pass it. It's what EJS is. EJS should never be used server-side with tainted/unsanitized input from an end-user. If you allow people to pass arbitrary code to it, it will run arbitrary code.

If a developer uses EJS this way, they are definitively using EJS wrong.

@mde mde closed this as completed Jan 22, 2021
@dominykas
Copy link
Contributor

While I agree with the sentiment that this is not exactly a security vulnerability in ejs as it is true that you should not be passing unsanitized user input to it, there still is a bug somewhere in there. Note the code - for whatever reason, the code inside options.filename string gets executed.

This only happens if options.compileDebug is set to true, which leads to me believe that there is a bug, likely here:

ejs/lib/ejs.js

Line 639 in 9f69c0a

+ '//# sourceURL=' + opts.filename + '\n';

Namely, if the filename contains a new line, then the second line escapes from the comment.

Not sure what the fix should be. A naive implementation should forbid \n in the filename, but probably this needs URI encoding or smth like that - not sure what the valid values are for this sourceURL comment, or how to escape these values exactly.

@mde
Copy link
Owner

mde commented Feb 6, 2021

Updated to prevent this in v3.1.6, now pushed to NPM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants