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

Consider adding a security warning? #53

Open
greggman opened this issue Sep 26, 2016 · 5 comments
Open

Consider adding a security warning? #53

greggman opened this issue Sep 26, 2016 · 5 comments

Comments

@greggman
Copy link

WebGL takes pretty extreme steps to prevent data leakage and other security issues.

It clears all memory allocated by GL. It checks for all out of bounds conditions. It also re-writes all shaders to make sure they don't exceed certain limits.

node-webgl seems to do none of this. This seems like a pretty big security issue

  • If user data is passed into node-webgl the server could easily be compromised.

Whether it's user shaders (like if you wanted to render shadertoy images on the server) or even textures and geometry data. If texture sizes are set by the user it could be easy to overwhelm the driver. If user indexed geometry is passed in it would allow reading all of GPU (and possibly CPU) memory by passing in out of range indices.

  • Bugs in server side code could expose user data.

Example: call gl.texImage2D with null then render the texture and call gl.readPixels or just add the texture to a framebuffer and call gl.readPixels. In either case you'll get uninitialized memory the contents of which could be passwords, ssh keys, photos, or other user data.

A bug in your GL code that seems to be working might actually be exposing uninitialized data

Another examples is client side arrays. WebGL doesn't allow them but node-webgl doesn't check which means attributes can read random places in memory.

With enough care it's certainly possible to use node-webgl for some use cases but I'm just wondering should there be some large warning in the README about these issues? I'd expect lots of devs to be unaware of them and just blindly add node-webgl to their project.

@DiThi
Copy link

DiThi commented Sep 27, 2016

Personally I expected this from this library, I hope nobody expects otherwise. I even skip the type checking wrapper and call the native functions directly. One of the reasons I use this lib instead of Electron or NW.js is to avoid all the security overhead.

Headless-gl may be what you're looking for. It's much more WebGL compilant, although I don't know if it's possible to render to a window with it.

@greggman
Copy link
Author

greggman commented Sep 27, 2016

Well given the first line in the readme is

This is a Node.JS port of WebGL

Yes, I'd expect people to think it's actually WebGL. That it's secure and compatible.

Thanks for the suggestion of headless-gl.

That said, the point of this issue is really probably that the readme should change. It should change to say this is not WebGL and it's also insecure.

@DiThi
Copy link

DiThi commented Sep 27, 2016

I agree, it's misleading. Consider making a pull request that updates the readme. I'll eventually send a pull request for adding more strict and more lax checking, for both use cases (performance, and security + debugging).

@trusktr
Copy link

trusktr commented Jul 28, 2017

Headless-gl may be what you're looking for. It's much more WebGL compilant, although I don't know if it's possible to render to a window with it.

headless-gl doesn't render, and it is slow (no GPU?), and is for integration testing.


It'd be awesome to have a standalone WebGL implementation (security features and everything), for use outside a browser.

@mikeseven
Copy link
Owner

mikeseven commented Jul 28, 2017 via email

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

4 participants