|
in reply to RFC: Perl-Critic policy: ProhibitInlineSystemArgs
Why? Please explain your reasoning?
Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
"Science is about questioning the status quo. Questioning authority".
In the absence of evidence, opinion is indistinguishable from prejudice.
|
Re^2: RFC: Perl-Critic policy: ProhibitInlineSystemArgs
by revdiablo (Prior) on Jul 01, 2006 at 04:23 UTC
|
Why? Please explain your reasoning?
Are you asking why to avoid passing arguments in the single-arg form of system or exec? It can be dangerous if passed unclean user input. There is a particularly relevant paragraph in the exec documentation (emphasis mine):
If there is more than one argument in LIST, or if LIST is an array with more than one value, calls execvp(3) with the arguments in LIST. If there is only one scalar argument or an array with one element in it, the argument is checked for shell metacharacters, and if there are any, the entire argument is passed to the system's command shell for parsing (this is /bin/sh -c on Unix platforms, but varies on other platforms). If there are no shell metacharacters in the argument, it is split into words and passed directly to execvp, which is more efficient.
Notice that in the case of a single argument with meta-characters, the system shell is called. The system shell will then interpret those metacharacters in the standard way, which can cause dangerous side effects. Consider a malicious user that passes "> $0" as part of the input. If this were executed in a unix shell with enough permissions, it could cause some major damage.
Of course it's always wise to validate user input, but the safest course of action is to not use the user input in a way that would be dangerous in the first place. In this case, the list forms of system and exec prevent the danger, because the shell is never invoked.
Note: I apologize in advance if I'm not answering the question you asked, but perhaps someone else will find the information useful if that is the case.
Update: apparently my efforts to avoid offending the ever-sensitive BrowserUk were in vain. I shall make a note to not even try in the future.
| [reply]
[d/l]
[select] |
|
|
Well. Ignoring that you are someone else, explaining someone elses reasoning, and indeed, the reasoning you explain is mostly not yours or the OP's; it will suffice to demonstrate that the OP's PerlCritic code does not even attempt to encapsulate the reasoning you've explained.
The difference between the rule coded and the salient part of your explaination--that is somewhat diluted in your explanation through lack of emphasis and entirely omitted from your quoted reference--is your second sentance:
It can be dangerous if passed unclean user input.
Even in that simple prerequisite there are two conditional parts that are entirely omitted from the rule as coded:
- can be
- unclean user input
Most Perl scripts are invoked by a user from a shell command line. The user has no need to deviously pass contaminated arguments into Perl scripts in order to have those scripts pass them on to the shell as they can do it directly.
Most system & exec invokations within Perl scripts do not pass unclean user input as a part of their arguments.
Omitting checks in the rule to allow scripts the benefits of reusing the facilities provided by system shells, where doing so does not conflict with security, is like passing a law that says 'noone is allowed to cross a road'.
It can be dangerous to cross a road without looking; but if the road is empty, or if the road is a private road currently closed to vehicular traffic; or if you are of sound mind, with reasonable eyesight, and no mobility impediments and you look first; it's probably okay to cross roads.
Imagine life if a law was imposed upon everyone that they could never cross roads because doing so can sometimes be dangerous if insufficient care is taken when doing so?
Of course, it would be quite difficult to encapsulate the logic required to verify whether a particular use of the single argument system or exec was likely to fall foul of the particular set of circumstances that would render that usage unsafe--but flagging all uses as unsafe because it is easier than performing the checks is not an acceptable alternative.
This is the metaphoric equivalent of imposing a blanket ban on eating out because some chefs are insufficiently aware of the rules for food preparation and might therefore give their client salmonella.
Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
"Science is about questioning the status quo. Questioning authority".
In the absence of evidence, opinion is indistinguishable from prejudice.
| [reply] |
|
|
You've got some good points, but you go a bit too far. There's always a way of coding things that doesn't run afowl of this stricture, and it doesn't involve extra work for the coder. It is also, as a nice side-effect, more efficent, and doesn't have a tendency to make things not work on odd filenames. Just do what the shell would yourself, and pass a list of multiple elements, or when you meant to have a single argument, don't put it in double-quotes.
Also, ain't nobody (sane) forcing you to use Perl::Critic. I'm not sure if it's lexical or "no"able, though...
Warning: Unless otherwise stated, code is untested. Do not use without understanding. Code is posted in the hopes it is useful, but without warranty. All copyrights are relinquished into the public domain unless otherwise stated. I am not an angel. I am capable of error, and err on a fairly regular basis. If I made a mistake, please let me know (such as by replying to this node).
| [reply] |
|
|
|
|
|
Re^2: RFC: Perl-Critic policy: ProhibitInlineSystemArgs
by radiantmatrix (Parson) on Jul 06, 2006 at 19:59 UTC
|
I can tell you why I suggested that capacity in the referenced node: very simply, it's way too easy to mess up (usually by a failure to anticipate a problem) when using the single-argument form.
When using a single-argument form of system or exec (i.e. system("$command $arg1 $arg2")), it's up to the programmer to escape those arguments properly for the shell. Which they might choose to do non-portably. The multiple-argument forms (i.e. exec($command, $arg1, $arg2, @other_args)) , though, don't have that issue: the shell escaping/quoting is handled under the covers.
Fewer bugs creep into code, and fewer portability problems arise, with the multiple-argument form. There's also very few instances where the muliple-argument form is any kind of hinderance. That's reason enough, IMO, to consider that form a best practice (even if it's a "low-severity" one when broken).
If that's not enough, though, there are some minor security issues. Tainted data being passed as arguments will be quoted for the shell (they won't be in the single-arg form), so there isn't a "Shell Code Injection" possibility. Maybe that doesn't come up most of the time, but considering the number of CGI and GUI applications I've seen that interpolate tainted (or very poorly untainted) user data into system calls, it's worth thinking about.
Essentially, this is the same argument as using the 3-arg form of open, and using prototypes for SQL statements with DBI. There are times when it's perfectly safe to use 2-arg open and to interpolate variables in SQL statements (esp. with proper untainting). But, 3-arg open and prototypes are best practices because it's just as easy to do it the preferred way, and it helps prevent common mistakes and problems. Same thing here.
I'll be the first to stand up and say that this isn't something Perl::Critic should gripe about by default, but it would be really nice to have it complain at lower severity levels.
| [reply]
[d/l]
[select] |
|
|
The multiple-argument forms (i.e. exec($command, $arg1, $arg2, @other_args)) , though, don't have that issue: the shell escaping/quoting is handled under the covers.
Yes, the multiple-argument forms don't have that problem but it's not because shell quoting is handled in any way, but because there isn't any shell involved at all, so no quoting needed. Parameters to exec are forwarded directly to the underlying kernel, who uses them without further modification.
| [reply] |
|
|
... the shell escaping/quoting is handled under the covers.
Fewer bugs creep into code, and fewer portability problems arise, with the multiple-argument form.
Complete red herring.
There's also very few instances where the muliple-argument form is any kind of hinderance.
Also not so
Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
"Science is about questioning the status quo. Questioning authority".
In the absence of evidence, opinion is indistinguishable from prejudice.
| [reply] |
|
|
I guess it all comes down to whether your goal in calling system is to provide shell-like capacities, or simply to execute foreign code. I stand by my statement that there are very few instances where the multiple-argument form is a hinderance: it is only a hinderance where you need to interact with the shell directly.
Perhaps your direct experience with system is largely in that category; mine, however, is not -- most times, I see calls to system that are simply execution of foreign code, and require no shell capabilities in the first place.
As for the response to the shell escaping/quoting, it's technically correct if pedantic. The point is, one bypasses the issue when using the multi-arg form -- how isn't entirely important, sorry if the over-simplification confused or annoyed anyone.
Because there are clearly times when the single-argument forms are appropriate, I would not (as I have said before) suggest that this be a high-severity warning. However, it would be a good thing to tickle at a lower severity, from the point of view of "are you sure you know what you're doing, here?"
We've both made our points, and I certainly respect your point of view. I think we'll just have to agree to disagreesort-of agree ;-) on this one.
Updates:
- 2006-07-07 : agree to disagree? not exactly... see replies
| [reply] |
|
|
|
|
