http://qs321.pair.com?node_id=559664


in reply to Re: RFC: Perl-Critic policy: ProhibitInlineSystemArgs
in thread RFC: Perl-Critic policy: ProhibitInlineSystemArgs

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.

<–radiant.matrix–>
A collection of thoughts and links from the minds of geeks
The Code that can be seen is not the true Code
I haven't found a problem yet that can't be solved by a well-placed trebuchet

Replies are listed 'Best First'.
Re^3: RFC: Perl-Critic policy: ProhibitInlineSystemArgs
by Hue-Bond (Priest) on Jul 06, 2006 at 20:20 UTC
    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.

    --
    David Serrano

[reply]
Re^3: RFC: Perl-Critic policy: ProhibitInlineSystemArgs
by BrowserUk (Patriarch) on Jul 06, 2006 at 23:54 UTC
    ... 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

      <–radiant.matrix–>
      A collection of thoughts and links from the minds of geeks
      The Code that can be seen is not the true Code
      I haven't found a problem yet that can't be solved by a well-placed trebuchet
[reply]
        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?"

        By that paragraph, we are exactly agreeing :)

        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]

In Section Meditations