1

Template actually comes from an index.html file from the same directory. Context can contain user input.

const context = { id: 1 };
const template = '<html><body>${id}</body></html>';
with (context) {
  return eval(`\`${template}\``);
}

I know about XSS protection.

I'm looking for examples of how to break this solution, is there any way a user input can run backend code?

  • Possible duplicate of [Why is using the JavaScript eval function a bad idea?](https://stackoverflow.com/questions/86513/why-is-using-the-javascript-eval-function-a-bad-idea) – Maxime Launois Apr 08 '19 at 17:38
  • 2
    two things you never want to use: with and eval :) – epascarello Apr 08 '19 at 17:39
  • I believe this is safe to run server-side, but you may still want to look into a templating module (I.E. nunjucks, pug) to have full templating functionality including template inheritance, includes, logic/loops, automatic escaping (XSS protection), etc. – Paul Apr 08 '19 at 17:59
  • ` ${ ['1', '2', '3'].map(e => `` ``).join('\n') }
    ${e}
    ` I have loops :)
    – Attila Varga Apr 08 '19 at 18:01
  • @AttilaVarga Sure, but that's not very readable (especially if you ever want to have nested loops/logic) and you're still missing some of the most important features... Without template inheritance you will need to repeat a lot of code on every template for instance. Even if you only have one template I highly recommend using a full-fledged templating engine instead of reinventing the wheel and adding features as you need them. – Paul Apr 08 '19 at 18:10
  • Using nunjucks is as simple as `npm install nunjucks` and then `return nunjucks.renderString( template, context );` where your template looks like `'{{id}}'` and your context is the same as the one in your question. – Paul Apr 08 '19 at 18:11
  • @Paulpro JSX works the same way, and this is not reinventing the wheel, it is the wheel itself. – Attila Varga Apr 08 '19 at 18:20
  • JSX is an overall bad idea. I can't fathom why anyone would want to mix a language as powerful and complex as JavaScript with their templates. – Paul Apr 08 '19 at 18:30
  • Because it shortens code, the only thing that relates to the number of bugs is LoC. It's ugly I agree. XML was a horrible idea back then. – Attila Varga Apr 08 '19 at 18:36

2 Answers2

1

Depending on how context is supplied with values, it may be possible to set context.template. This would change the variable identifier template to refer to the property context.template, and have that property's value passed into eval.

Therefore, you could have server-side code execution via a context property like

template: "${alert(1)}"

Alternately, if a context property could be given a function value, then setting context.eval would allow immediate execution of that function. (This is less likely to be feasible, however, since user input is much more likely to be treated universally as a string.)

You could avoid both of these issues by doing delete context.template; delete context.eval; before entering the with block.

apsillers
  • 112,806
  • 17
  • 235
  • 239
  • Ta, it definitely broke it :D and without with? (The context is provided by the code though, keys can contain user input) – Attila Varga Apr 08 '19 at 18:12
  • @AttilaVarga Assuming you dropped `with` and wrote your templates like `${context.id}`, etc? I think that would be safe (but I can only ever be perfectly sure that something is breakable, never perfectly sure that it's safe!). The vector I'd try without `with` is to get an interesting value in `context.__proto__` to see its properties: e.g., make `context.__proto__ = global` and find a template with a `context.process` access. But if I'm only allowed to store simple string values, I don't think that will allow me to do anything nasty or interesting with `__proto__`. – apsillers Apr 08 '19 at 18:55
0

It is never a good idea to use eval.

If you want to interpolate the string you can use template literals.

Even then you should be sanitizing use input to avoid XSS. A simple way to sanitize a string is to read the input as text which would escape the input and then dump the content into the template placeholder.

const context = { id: 1 };
const template = `<html><body>${getHTMLSafeText(context.id)}</body></html>`;

function getHTMLSafeText(content) {
    let div = document.createElement('div');

    div.textContent = content;

    return div.innerHTML;
}
Sushanth --
  • 55,259
  • 9
  • 66
  • 105
  • 1
    There is no feature in js to parse template literals apart from eval and yeah I use XSS protection if I print HTML. – Attila Varga Apr 08 '19 at 17:37
  • I'm looking for examples of how to break this solution, is there any way a user input can run backend code? – Attila Varga Apr 08 '19 at 17:41
  • "There is no feature in js to parse template literals apart from eval"? That's patently false. Run the following in the console: ``console.log(`The date is ${new Date()}`)`` – Heretic Monkey Apr 08 '19 at 17:42
  • Sure. You could do it that way ( what do you mean by `evaluate` here ? `eval` ?) – Sushanth -- Apr 08 '19 at 17:48
  • You have one file: index.html and another one index.js. index.html contains a template literal (without quotes) index.js contanis `const template = fs.readFileSync('index.html')` Try to evaluate it without eval. You can say, I could use a templating lib like handlebar or jinja2 but then I could not use javascript as a templating language. I could also use react but it's too fat for a simple service returning an HTML file. – Attila Varga Apr 08 '19 at 17:52