-
Notifications
You must be signed in to change notification settings - Fork 28.9k
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
Probable security issue, leaking file descriptors into terminal process #58814
Comments
If I'm understanding you correctly, I'm not sure how this sort of thing could happen. A pty is forked here: And the process is launched here: You seem more familiar with what's going on so if you have any insights into how this could happen that would be helpful. |
That's the problem here, there are multiple ways to do that. The cleanest way would be that all opened files would be opened with the There are some "workarounds", one is to manually loop through all possible open filedescriptors ( I could probably implement the workaround and send a pull request if wanted, but i am not set up for building vscode completely here. |
/cc @jerch |
@Tyriar Imho a serious bug in node-pty, it might even leak the fds to subprocesses (depends on the shell, imho zsh wipes them while Only chance we have imho is looping through it. Id even vote for closing any fd > 2 before exec, since thats all a pty connection needs. Imho there was even an old issue for it. |
Bash seems to leak them at least. Not using z-shell or something other here so I can't comment on this. |
Edited the typo above. @dunkelstern If you like to write a patch, node-pty is the place to go. (You dont have to build the full vscode for it). |
I am looking to send a patch on the weekend. So looping through all descriptors is fine in your opinion too? |
@dunkelstern Not sure if it will break other things, as it would also close fds that are leaked by purpose (my process tracker relied on that, some debugger might also). I guess only a member of vscode can tell if they utilize this behavior to some degree. |
Any idea whom/where to ask for something like that? I mean i could try out some things on my own (js, python, some native debuggers and the extensions I use come to mind) but of course i can not test everything on my own. |
Testing all one by one is, well tedious, errorprone and might lead to false assumptions. |
I don't think this is an issue because the pty launched by node-pty by design is always launched at the same permission level as what it's launched from, therefore the pty can do anything of the parent process anyway. As an action item I'll make a note on the node-pty readme about what not to do microsoft/node-pty#230 |
Issue Type: Bug
When running a process from the integrated terminal it seems some file-descriptors are inherited. For example my bash has some open file-descriptors to various asar and font files (and a lot of shared memory!).
To reproduce (on Linux) use the following command directly in the integrated Terminal:
ls -l /proc/$$/fd
This lists all open files for the current process (the bash) from the
/proc
file system. If you use the-l
switch it shows to which file the file-descriptor points too.Example output (in summary because of long output)
I can see no direct issue for now but it is possible to access files or shared memory this way and probably crash VScode this way. Additionally it is completely unexpected behaviour (i was stumped why my single-file python process had over 100 open file descriptors).
VS Code version: Code 1.27.1 (5944e81, 2018-09-06T09:21:47.222Z)
OS version: Linux x64 4.15.0-34-generic
Ubuntu 18.04
System Info
checker_imaging: disabled_off
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: disabled_software
rasterization: disabled_software
video_decode: unavailable_software
video_encode: unavailable_software
webgl: enabled
webgl2: enabled
Extensions (26)
The text was updated successfully, but these errors were encountered: