miketheprogrammer / go-thrust

Cross Platform UI Kit powered by Blink/V8/Chromium Content Lib
MIT License
445 stars 34 forks source link

Security: Fix Possible Symlink race in package spawn #7

Open miketheprogrammer opened 9 years ago

miketheprogrammer commented 9 years ago

https://en.wikipedia.org/wiki/Symlink_race

I was enlightened to the fact that there may be a Symlink Race condition in our auto bootstrapping methods. Need to research the subject and apply a fix.

miketheprogrammer commented 9 years ago

This is linked to #12

tehbilly commented 9 years ago

If we're paranoid we can do a hash check of the linked file afterwards. What is the actual surface area of this risk?

miketheprogrammer commented 9 years ago

I dont know, @tv42 tv` in irc #go-nuts mentioned it. I think for obvious reasons many go programmers are much more knowledgable about certain security flaws and therefore would like to see code that at least try to move in the right direction.

I think but Im not sure, that this was fixed in the provisioner merge. I stopped using the global temp directories and used the basedirectory set by the user. This is also only a risk with the default provisioner anyway which is really just a helper for devs to get started. Any final distro should have the binaries packed in.

miketheprogrammer commented 9 years ago

I think we should definitely do a hash check of the Thrust Exe if the Default Provisioner/Spawner are used. Dont want to accidentally execute an unknown executable in privileged mode.

I think the symlink race is handled however by not using the tmp dir or any globally accessible dir. Also, maybe we can avoid even downloading the file to the disk, by somehow mocking the file API. @tehbilly Do you know how to create an io.ReaderAt with a []byte, if we could do that then we could never even touch the fs execpt for the final unzipped files.

cbandy commented 9 years ago

Do you know how to create an io.ReaderAt with a []byte

http://golang.org/pkg/bytes/#Reader

miketheprogrammer commented 9 years ago

Thanks for the link. Will probably try to get this in soon, unless someone wants to do a PR. Really would like to get the symlink race fixed.