Inspector v2 number/expression parsing not safe

Inspector v2’s number input fields do not work properly in contexts in which eval is not permitted.

EvaluateExpression uses the Function() call as a shortcut to allow arbitrary expressions to be evaluated. However, if eval is not permitted, this always fails with NaN being returned as the fallback.

Minimally, I believe this should return something like Number.parseFloat(rawValue) or Number(rawValue) as the fallback. However, there are also potential security concerns at play here that are worth further thought by the team. For example, you can input things like alert(1) or (()=>{while(1);})() into the page to have some undesirable effects.

cc @ryantrem @georgie

Hey @kvbh, thanks for the heads up! I see the issue you are pointing out of always relying effectively on eval, even for a constant, and we’ll definitely fix that.

As far as arbitrary expressions being unsafe, I’m less sure. This is always direct user input, and you could type the same expression in the debug console and get the same result. Open to other perspectives, but we added this feature based on user requests, and the only alternative I see to support it is to either write or depend on a math expression evaluator library, which we probably wouldn’t want to do.

I agree that it’s likely not really a security concern or anything like that, as anyone getting into the inspector is probably technical enough to know how to do dangerous things. The particular issue I face is that on a website with a Content-Security-Policy that disallows eval, the input fields don’t work at all, even with simple inputs. I would be satisfied with just having a fallback that parses the input as a simple float number.

Fix SpinButton number expression parsing under strict CSP by ryantrem · Pull Request #18491 · BabylonJS/Babylon.js