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

Please make claygl CSP compatible when "script-src unsafe-eval" is used #133

Open
Jas2042 opened this issue Mar 5, 2022 · 7 comments
Open

Comments

@Jas2042
Copy link

Jas2042 commented Mar 5, 2022

I am using HTTP Header: "Content-Security-Policy" without script-src "unsafe-eval"
(See unsafe_eval_expressions)

claygl is incompatible with theses settings due to the function "tryConvertExpr(string)"

It uses Function() which is disallowed unless using "unsafe-eval"

@Jas2042 Jas2042 changed the title Please make claygl CSP compatible when " script-src unsafe-eval" is used Please make claygl CSP compatible when "script-src unsafe-eval" is used Mar 5, 2022
@qcp
Copy link

qcp commented Apr 17, 2023

Still important

@Jas2042
Copy link
Author

Jas2042 commented Apr 18, 2023

Related: ecomfe/vue-echarts#707

@setvik
Copy link

setvik commented Oct 5, 2023

Haven't dug into the code deep enough yet, but chatGPT suggested the following template for an approach to replacing new Function(...):

"One way to eliminate the new Function call and still evaluate the expression is to employ a safe expression evaluator. Here, we can make use of a simplistic evaluator that handles basic arithmetic operations, which may serve many use cases.

Here's a version of your tryConvertExpr function that uses this approach:

function tryConvertExpr(string) {
    var exprRes = /^expr\((.*)\)$/.exec(string);
    if (exprRes) {
        var expression = exprRes[1];

        return function(width, height, dpr) {
            return safeEval(expression, { width, height, dpr });
        };
    }
}

function safeEval(expr, vars) {
    // Simple expression evaluator. Supports basic arithmetic operations.
    // Note: This evaluator is rather simplistic. Depending on the range of
    // expressions you expect, you may want to expand its capabilities or use
    // a third-party library.

    function evaluate(node) {
        switch (node.type) {
            case "Literal": return node.value;
            case "Identifier": return vars[node.name];
            case "BinaryExpression": {
                let left = evaluate(node.left);
                let right = evaluate(node.right);
                switch (node.operator) {
                    case "+": return left + right;
                    case "-": return left - right;
                    case "*": return left * right;
                    case "/": return left / right;
                    default: throw new Error(`Unsupported operator: ${node.operator}`);
                }
            }
            default: throw new Error(`Unsupported node type: ${node.type}`);
        }
    }

    let ast;
    try {
        // Using the Esprima library to parse the expression into an AST.
        // You will need to include the Esprima library for this to work.
        ast = esprima.parse(expr).body[0].expression;
    } catch (e) {
        throw new Error("Invalid expression.");
    }
    
    return evaluate(ast);
}

In this version:

  • The safeEval function evaluates an expression safely without running arbitrary JavaScript. This is achieved by parsing the expression into an abstract syntax tree (AST) using the esprima library and then evaluating the AST.

  • The tryConvertExpr function returns a function that, when called with width, height, and dpr, evaluates the expression in the context of those variables.

Note: You'll need to include the Esprima library for this code to work, as it is used to parse the expressions.

Ensure you understand the limits of this evaluator. Depending on the complexity of the expressions you need to handle, you might need to expand the evaluator or consider other libraries or techniques."

@Jas2042
Copy link
Author

Jas2042 commented Oct 5, 2023

A quick look and where tryConvertExpr() is called seems to indicate the returned Function() is dynamically called on the 3D Scene. (Not really sure tho)

The code above would not be very performant but might be good enough.
It depends. Perhaps someone who understands the use case can comment.
(Give the age of the codebase I am not very hopeful)

If something like the above is used then we only want to create the ast when the new function is created and not every time it's called.

function evaluateConvertExpr(node, vars) {
    switch (node.type) {
        case "Literal": return node.value;
        case "Identifier": return vars[node.name];
        case "BinaryExpression": {
            const left  = evaluateConvertExpr(node.left, vars);
            const right = evaluateConvertExpr(node.right, vars);
            switch (node.operator) {
                case "+": return left + right;
                case "-": return left - right;
                case "*": return left * right;
                case "/": return left / right;
                default: throw new Error(`Unsupported operator: ${node.operator}`);
            }
        }
        default: throw new Error(`Unsupported node type: ${node.type}`);
    }
}

function tryConvertExpr(string) {
    const exprRes = /^expr\((.*)\)$/.exec(string);
    if (exprRes) {
        try {
            // Using the Esprima library to parse the expression into an AST.
            // You will need to include the Esprima library for this to work.
            const ast = esprima.parse(exprRes[1]).body[0].expression;
            return function(width, height, dpr) {
                return evaluateConvertExpr(ast, { width, height, dpr });
            };
        } catch (e) {
            throw new Error("Invalid expression.");
        }
    }
}

Really needs the spec / docs for what is allowed in the expression string.
If ONLY "width, height, dpr" are allowed in the expression then the above works fine.

@Jas2042
Copy link
Author

Jas2042 commented Oct 5, 2023

Also an approach like the following libs use may be feasible:

@setvik
Copy link

setvik commented Oct 5, 2023

The two functions that call tryConvertExpr are:

function createSizeSetHandler(name, exprFunc) {
    return function (renderer) {
        // PENDING viewport size or window size
        var dpr = renderer.getDevicePixelRatio();
        // PENDING If multiply dpr ?
        var width = renderer.getWidth();
        var height = renderer.getHeight();
        var result = exprFunc(width, height, dpr);
        this.setParameter(name, result);
    };
}

function createSizeParser(name, exprFunc, scale) {
    scale = scale || 1;
    return function (renderer) {
        var dpr = renderer.getDevicePixelRatio();
        var width = renderer.getWidth() * scale;
        var height = renderer.getHeight() * scale;
        return exprFunc(width, height, dpr);
    };
}

both of which only pass in the width, height, and dpr into the generated function.

For my simple use case (scatterGL), here were the unique expressions generated:

[width * 1.0 / 16, height * 1.0 / 16] 
[width * 1.0 / 16, height / 16] 
[width * 1.0 / 2, height * 1.0 / 2] 
[width * 1.0 / 2, height / 2]
[width * 1.0 / 2, height / 2] 
[width * 1.0 / 32, height / 32] 
[width * 1.0 / 4, height * 1.0 / 4] 
[width * 1.0 / 4, height / 4] 
[width * 1.0 / 8, height * 1.0 / 8] 
[width * 1.0 / 8, height / 8] 
[width * 1.0, height * 1.0] 
height * 1.0
height * 1.0 / 16
height * 1.0 / 2
height * 1.0 / 32
height * 1.0 / 4
height * 1.0 / 8
width * 1.0
width * 1.0 / 16
width * 1.0 / 2
width * 1.0 / 32
width * 1.0 / 4
width * 1.0 / 8

Not sure if the two types shown here (single arithmetic expression and array of arithmetic expressions) is exhaustive.

@setvik
Copy link

setvik commented Oct 5, 2023

Took a shot at a PR, tested locally to ensure it produces the exact same output / results as the existing tryConvertExpr function.

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