-
Notifications
You must be signed in to change notification settings - Fork 916
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
Replace Tinymath with math.js #4492
Changes from all commits
c0e737e
efd1972
7fd7342
3ce717f
55a1c06
2887b18
e4d60db
a46dc9a
852cf06
aca53d5
46f3313
c6a59b0
6fd0370
e50971d
f5f0a82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { create, all } from 'mathjs'; | ||
|
||
const math = create(all); | ||
export const evaluate = math.evaluate; | ||
export const MATH_THROW_MSG = 'Math.js function is disabled'; | ||
|
||
math.import( | ||
{ | ||
import() { | ||
throw new Error(MATH_THROW_MSG); | ||
}, | ||
createUnit() { | ||
throw new Error(MATH_THROW_MSG); | ||
}, | ||
evaluate() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this evaluate() function different than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is the same So it is possible to do: |
||
throw new Error(MATH_THROW_MSG); | ||
}, | ||
parse() { | ||
throw new Error(MATH_THROW_MSG); | ||
}, | ||
simplify() { | ||
throw new Error(MATH_THROW_MSG); | ||
}, | ||
derivative() { | ||
throw new Error(MATH_THROW_MSG); | ||
}, | ||
}, | ||
{ override: true } | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we disable the following math.js functions due to security issues? How do you know these are the only functions that we need to disable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions are marked as unsafe by the mathjs documentation https://mathjs.org/docs/expressions/security.html
A user could try to inject malicious JavaScript code via the expression parser. The expression parser of mathjs offers a sandboxed environment to execute expressions which should make this impossible. It’s possible though that there are unknown security vulnerabilities, so it’s important to be careful, especially when allowing server side execution of arbitrary expressions.
The expression parser of mathjs parses the input in a controlled way into an expression tree or abstract syntax tree (AST). In a “compile” step, it does as much as possible preprocessing on the static parts of the expression, and creates a fast performing function which can be used to evaluate the expression repeatedly using a dynamically passed scope.
The parser actively prevents access to JavaScripts internal eval and new Function which are the main cause of security attacks. Mathjs versions 4 and newer does not use JavaScript’s eval under the hood.