miguelgrinberg/Flask-Moment

Eval() is Evil ;) (unsafe Content-Security-Policy)

e2jk opened this issue · 2 comments

e2jk commented

When activating Content-Security-Policy (CSP) with script-src to allow for local scripts and cdnjs.cloudflare.com where the moment.js gets downloaded from by Flask-Moment, the following error gets triggered and further execution of moment.js gets blocked:

image

This is the line in question:

$(elem).text(eval('moment("' + $(elem).data('timestamp') + '").' + $(elem).data('format') + ';'));

One way to get around this would be to allow for unsafe-eval in the CSP, but that is not a good idea since that means that any script used on the website can use eval (and thus any potential malicious script). The right way would be to replace the eval() call in the code.

(in case the reference needs explaining: the title of the issue comes from Douglas Crockford's JavaScript: The Good Parts. Another bit about why Eval is Evil: https://javascriptweblog.wordpress.com/2010/04/19/how-evil-is-eval/ )

e2jk commented

I just did it again, opening an issue without noticing #43 and #45.
However, reviewing the discussion, I still think a PR (I'll see if I can help with that) would make sense, to be able to use include_moment() just as the library is thought to be used.
I see the past discussion mentions the fact that this specific usage of eval seems unharmful (which I agree), but the problem is that if we want to use include_moment() and CSP, it means using unsafe-eval for all the scripts on the webpage, and thus potentially allowing other libraries/malicious scripts usage of eval()...

e2jk commented

Looking a bit more at the code, it seems to me that this is solvable pretty easily.
Right now the code works by generating a <span> tag with a data-timestamp and a data-format attribute.
The data-timestamp is fine (and it's content can later be retrieved and passed to the moment() function without needing eval()), but the issue comes from the data-format attribute which contains both the function and the argument(s) that need to be performed on the Moment object.

It seems to me that by separating the content of what goes now in data-format we can get rid pretty easily of the call to eval(), these would be the changes to apply to the _render() Python function:

  • remove data-format
  • add a new data-function that would simply be the name of the function in string format (like "format", "fromNow" or "calendar")
  • add a new data-argument that would be the argument passed when the function is format, fromNow, or from (no need for arguments when the function is calendar(), valueOf() or unix())
  • one last data-from-timestamp when the function is "from", since the call to from needs a second attribute to indicate the first timestamp

Then in the JavaScript code that gets generated at the bottom of include_moment():

  • call moment($(elem).data('timestamp')) to get the Moment representation of the timestamp
  • and then a Switch statement based on $(elem).data('function') that call the appropriate Moment JavaScript function with the $(elem).data('argument') argument (and the extra second argument $(elem).data('from-timestamp') when the function is "from"