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

fix(eval): improve security of safe-eval #233

Merged
merged 2 commits into from
Nov 17, 2024

Conversation

80avin
Copy link
Contributor

@80avin 80avin commented Nov 15, 2024

PR description

  • block reading properties 'constructor', 'proto', 'defineGetter', 'defineSetter' if they are not owned by the object.
  • allow only expected variables in global scope ( removing constructor, proto, etc from global scope )
  • Remove previous patches to fix security issues. Ensure no breakage by adding unit tests

Checklist

  • - Added tests
  • - Ran npm test, ensuring linting passes
  • - Adjust README documentation if relevant

@80avin 80avin marked this pull request as ready for review November 15, 2024 21:02
* block reading properties 'constructor', '__proto__', '__defineGetter__', '__defineSetter__' if they are not owned by the object.
* allow only expected variables in global scope ( removing constructor, __proto__, etc from global scope )
* Remove previous patches to fix security issues. Ensure no breakage by adding unit tests
@80avin
Copy link
Contributor Author

80avin commented Nov 15, 2024

@brettz9 Please review.
Hope this will fix most of the security issues.

test/test.safe-eval.js Outdated Show resolved Hide resolved
rebuild docs using `pnpm run license-badges && pnpm run build-docs && pnpm run lint && pnpm run test`, remove unnecessary changes in test/test.safe-eval.js and badges/license-badge-dev.svg
@brettz9
Copy link
Collaborator

brettz9 commented Nov 17, 2024

@80avin : Please check your email . Thanks!

@brettz9 brettz9 merged commit 73ad72e into JSONPath-Plus:main Nov 17, 2024
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

Successfully merging this pull request may close these issues.

2 participants