fix: CSP Security hole in tryConvertExpr (new Function(...)) #139
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jas2042
suggested changes
Oct 6, 2023
| catch (e) { | ||
| throw new Error('Invalid expression.'); | ||
| } | ||
|
|
| }; | ||
|
|
||
| // Try run t | ||
| try { |
|
|
||
| const func = function(width, height, dpr) { | ||
| const context = { width, height, dpr }; | ||
| const mexp = new Mexp() |
There was a problem hiding this comment.
If the expressions are as simple as the bug implies then rather than ast and all the overhead of generating it perhaps simple str substitution and split could do the job.
This would remove the need for esprima and the astToMathExpression() function
This could be better but something like:
expression = expression.replaceAll('width", width).replaceAll('height", height).replaceAll('dpr", dpr).replaceAll("[", "").replaceAll("]", "");
expressions = expression.split(",")
if (expressions.length === 1) {
return mexp.eval(expressions[0]);
}
return [ mexp.eval(expressions[0]), mexp.eval(expressions[1]) ];
Probably needs some exception handling as well
|
|
||
| const func = function(width, height, dpr) { | ||
| const context = { width, height, dpr }; | ||
| const mexp = new Mexp() |
There was a problem hiding this comment.
Can the const mexp value move out of the func scope so in instantiated for each call?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses the CSP security hole identified in the issue linked below and allow ClayGL to be used on sites with strict Content Security Policies:
Please make claygl CSP compatible when "script-src unsafe-eval" is used.