[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCH] Prevent Perl exec function from ever interpreting commands as shell



On Tue, 2023-06-13 at 11:56 +0200, Guillem Jover wrote:

> This change would break the current semantics for this option, as it
> is specified to take a "command-string". Perhaps the man page needs to
> be improved to make all this more clear. Where did you find this to be
> a problem? (If we'd really wanted to change the semantics that would
> require an explicit deprecation cycle, first try to catch affected usage
> and warn, then emit errors on those and use the new semantics, etc.,
> but it's not clear to me this would be the better option.)

I was writing this mail when I stumbled upon the behaviour:

https://lists.debian.org/msgid-search/da17fe19bcc05ba1032e5d81112bb6715825a6a7.camel@debian.org

Ever since writing an old blog post about shell metacharacter injection
attacks, I am always looking for situations where shell metacharacter
injection could lead to unwanted or unexpected behaviour and I vastly
prefer consistent C-style exec behaviour where there is no chance of
the shell being involved and messing around with the command. IMO the
decision to use shell should always be on the user and require the user
to explicitly call the shell and write a shell script (or use `sh -c`).

https://bonedaddy.net/pabs3/log/2014/02/17/pid-preservation-society/

So I think that at minimum, the current behaviour should be documented.

Perhaps it should go through the deprecation cycle you mention too.

> While applying this would not harm, it should also not be needed as we
> always pass at leasth three three elements in @cmds. Well it might on
> Windows, but I don't think it is generally supported anyway.

Yes I understood that, but I thought it would be best to change the
style of exec usage to be consistent throughout dpkg. Dpkg::IPC uses
this style, dpkg-architecture should and the whole codebase should too.

-- 
bye,
pabs

https://wiki.debian.org/PaulWise

Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: