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

OpenCV.js Security Issue #3159

Open
ozen opened this issue May 6, 2021 · 4 comments
Open

OpenCV.js Security Issue #3159

ozen opened this issue May 6, 2021 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@ozen
Copy link
Contributor

ozen commented May 6, 2021

We were having problems with CSP while loading opencv.js. A quick look revealed that opencv.js is loaded with an unsafe eval, and the ESLint error is suppressed in the code, as seen below:

https://github.com/openvinotoolkit/cvat/blob/df175a856179f1c31ac2b15c80d98986a3d35acf/cvat-ui/src/utils/opencv-wrapper/opencv-wrapper.ts#L63-L65

So loading the script is blocked in the browser due to the unsafe eval.

We wonder why loading opencv.js is implemented this way. I'm not a frontend engineer, but all resources say the same thing: this is a terrible practice, eval is evil, it should not be used, and there is no reason to use it.

@nmanovic nmanovic added the enhancement New feature or request label May 6, 2021
@nmanovic nmanovic added this to the Backlog milestone May 6, 2021
@nmanovic
Copy link
Contributor

nmanovic commented May 6, 2021

@bsekachev , do you think we can find an alternative way to load OpenCV dynamically?

@bsekachev
Copy link
Member

@nmanovic

There are three ways to upload script dynamically:

  1. eval - nothing really dangerous when using eval on frontentd because any user can anyway perform any code via console.
  2. new Function('stringified code') - the same like eval, but without accessing to local scope
  3. add <script src=".." /> to DOM - there were some issues with it during implementation, but I do not remember exactly what.

What is more, we already use new Function structure in cvat-ui/src/utils/enviroment.ts when setup cvat.org web analytics.
Since script is downloaded from the main server, I am not very sure why CSP is a problem, need to investigate.

@ozen
Copy link
Contributor Author

ozen commented May 6, 2021

Serving over HTTPS section of the installation manual during v1.2.0 included Content-Security-Policy header without unsafe-eval in nginx conf.

https://github.com/openvinotoolkit/cvat/blob/37d82f90052ab26c584dfb2c87a4623cb22e0ff4/cvat/apps/documentation/installation.md#L576

@daniel-dsouza
Copy link

daniel-dsouza commented May 27, 2021

I could not figure out how to properly configure NGINX to make Chromium happily load opencv.js, so I had to disable https.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants