arthwang / vsc-prolog

A VS Code extension that provides language support for prolog
MIT License
97 stars 21 forks source link

Arbitrary code execution #31

Open edom opened 5 years ago

edom commented 5 years ago

Should I report it here? Do you have other policies for reporting security vulnerabilities?

arthwang commented 5 years ago

Vulnerabilities are automatically reported by github.com, AFAIK. What do you care about it?

Erik Dominikus notifications@github.com 于2018年12月1日周六 上午2:39写道:

Can I report it here? Do you have other policies for reporting security vulnerabilities?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/arthwang/vsc-prolog/issues/31, or mute the thread https://github.com/notifications/unsubscribe-auth/AH2UssIG3XSLLfk4-gaRUdEH8RQ2dcJqks5u0XtugaJpZM4Y8Lvr .

edom commented 5 years ago

Github only finds vulnerabilities with the JavaScript/TypeScript part. The vulnerability I'm reporting is due to the interaction between this plugin and the Prolog interpreter. Github cannot detect this vulnerability. I'll report it here then.

The problem: arbitrary code execution

Imagine this file:

% doom.pro
:- shell('touch ~/doomed').

I loaded that file into VSCode with VSC-Prolog plugin enabled, and a file named doomed appeared in my home directory. Someone could replace that command with anything more dangerous, such as rm -rf /, download malware from the Internet, etc.

The problem is: checking a Prolog source file requires executing it.

The solution?

I'm still not sure what the best solution is: whitelisting, sandboxing, or something else.

Perhaps we should warn the user to not open untrusted source files?

(In practice, one rarely opens untrusted source code files.)

arthwang commented 5 years ago

Thank you for your information. It's really a risk to load untrusted files since VSC-Prolog provides syntax check upon source files loaded. Maybe I can disable it as default function and professional users have to enable it in settings. But it seems weird without any information to show in editor for files with errors or warnings. Any suggestions are appreciated.

Erik Dominikus notifications@github.com 于2018年12月2日周日 下午9:00写道:

Github only finds vulnerabilities with the JavaScript/TypeScript part. The vulnerability I'm reporting is due to the interaction between this plugin and the Prolog interpreter. Github cannot detect this vulnerability. I'll report it here then. The problem: arbitrary code execution

Imagine this file:

% doom.pro :- shell('touch ~/doomed').

I loaded that file into VSCode with VSC-Prolog plugin enabled, and a file named doomed appeared in my home directory. Someone could replace that command with anything more dangerous, such as rm -rf /, download malware from the Internet, etc.

The problem is: checking a Prolog source file requires executing it. The solution?

I'm still not sure what the best solution is: whitelisting, sandboxing, or something else.

Perhaps we should warn the user to not open untrusted source files?

(In practice, one rarely opens untrusted source code files.)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/arthwang/vsc-prolog/issues/31#issuecomment-443505763, or mute the thread https://github.com/notifications/unsubscribe-auth/AH2UsmgoceRAcSdg4GdmdLuJoPUGDh8iks5u087WgaJpZM4Y8Lvr .

edom commented 5 years ago

Maybe I can disable it as default function and professional users have to enable it in settings.

That sounds good.

But it seems weird without any information to show in editor for files with errors or warnings.

Even without error-checking, the plugin is still very valuable if it can do (1) syntax highlighting and (2) autocompletion/IntelliSense/Ctrl+Space. But are these features possible without executing the Prolog file?

Any suggestions are appreciated.

I think the cleanest solution is that the Prolog interpreters should whitelist directives. For example, SWI-Prolog has sandboxing, but this is not standard Prolog. I also understand that we may not have the time to add sandboxing to all Prolog interpreters.

I think the second cleanest solution is to write our own parser without depending on a Prolog interpreter, so that we can parse the file without executing it. But writing a parser takes time, and I understand that we may not have the time to write a parser.

Therefore, I think your original suggestion ("disable it as default function and professional users have to enable it in settings") is good.

arthwang commented 5 years ago

Maybe I can disable it as default function and professional users have to enable it in settings.

That sounds good.

But it seems weird without any information to show in editor for files with errors or warnings.

Even without error-checking, the plugin is still very valuable if it can do (1) syntax highlighting and (2) autocompletion/IntelliSense/Ctrl+Space. But are these features possible without executing the Prolog file?

Only lintering function is depended on executing swipl. Disabling lintering upon file loaded doesn't affect other functionalities. OnType and OnSave lintering should remain there.

Any suggestions are appreciated.

I think the cleanest solution is that the Prolog interpreters should whitelist directives. For example, SWI-Prolog has sandboxing, but this is not standard Prolog. I also understand that we may not have the time to add sandboxing to all Prolog interpreters.

I think the second cleanest solution is to write our own parser without depending on a Prolog interpreter, so that we can parse the file without executing it. But writing a parser takes time, and I understand that we may not have the time to write a parser.

Therefore, I think your original suggestion ("disable it as default function and professional users have to enable it in settings") is good.

That's true. We can't spend too much time on such project without enough financial supports.

step21 commented 3 years ago

@edom This is also a general Problem with VSCode / all IDEs that try to do fancy stuff. I don't really know any way around it.

edom commented 3 years ago

@step21 Indeed this difficulty also applies to all languages that were designed with the tacit assumption that the input source code is trustworthy (and thus the code may contain "fancy stuff" such as arbitrary macro expansion before run-time).

I also see no easy way around it, other than trusting all code (or disabling the parts of the plugin that require "fancy stuff"), doing a regular external backup, and testing the backup regularly.

Perhaps I was just too paranoid.