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

Don't execute code for the perl linter #1186

Closed
trinitum opened this issue Dec 1, 2017 · 9 comments

Comments

Projects
None yet
2 participants
@trinitum
Copy link

commented Dec 1, 2017

In some cases syntax check may result in executing the code, which might be a problem if somebody opens a file from untrusted source, here's an example with Perl:

$ echo test > /tmp/foobar.txt
$ ls /tmp/foobar.txt
/tmp/foobar.txt
$ echo "BEGIN { unlink '/tmp/foobar.txt' }" > unlink.pl
$ vim unlink.pl # just open the file and exit
$ ls /tmp/foobar.txt
ls: cannot access /tmp/foobar.txt: No such file or directory

wouldn't it make sense to keep all the checkers disabled by default and make users to explicitly enable those that they need?

@w0rp

This comment has been minimized.

Copy link
Owner

commented Dec 1, 2017

For which linter? Linters typically shouldn't execute much code by default, and those that do should be disabled by default.

@w0rp w0rp added the triage label Dec 1, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

commented Dec 1, 2017

All linters will remain enabled by default, except for those which might actually cause problems.

@trinitum

This comment has been minimized.

Copy link
Author

commented Dec 1, 2017

it's perl -c, it executes BEGIN blocks

@w0rp w0rp changed the title Some code can be actually executed Don't execute code for the perl linter Dec 2, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

commented Dec 2, 2017

Okay, I updated the issue.

@w0rp w0rp added enhancement and removed triage labels Dec 2, 2017

@w0rp w0rp closed this in f5fc746 Dec 2, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

commented Dec 2, 2017

Now -w is the default for perl instead of -c. The documentation explains the security problems and tells you how to change the option if you want to. That will be the default now and forever because it's safer.

@trinitum

This comment has been minimized.

Copy link
Author

commented Dec 2, 2017

I'm sorry, but that's completely wrong. -w is essentially the same as -Mwarnings it enables warnings, so perl -w file.pl will execute the whole script, not just BEGIN and CHECK blocks as perl -c. The previous implementation was correct, perl -c is what you run if you want to check the syntax of the perl script, but by design it executes BEGIN and CHECK blocks, there's simply no way to check the syntax of perl script without executing them. And that's fine when you are using it to check the syntax of your scripts, but if script comes from untrusted source it might be a problem, which is why I suggested you to disable it by default. My guess is that average person, not familiar with Perl, doesn't expect that opening a perl script in their favorite text editor may result in executing some code.

@w0rp

This comment has been minimized.

Copy link
Owner

commented Dec 2, 2017

Okay then, I'll disable it completely by default.

w0rp added a commit that referenced this issue Dec 2, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

commented Dec 2, 2017

Try being more respectful when you speak.

w0rp added a commit that referenced this issue Dec 2, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

commented Dec 2, 2017

Now it's disabled by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.