Hi!
On Wed, 2023-06-14 at 09:49:41 +0800, Paul Wise wrote:
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.
Attached is the man page update I've got queued locally, but I'm happy
to clarify further if you think that would be insufficient.
Perhaps it should go through the deprecation cycle you mention too.
I checked on sources.d.n, and AFAIR only saw a couple of potentially problematic instances. But this might be affecting local scripts and
tooling. I'll ponder about it, or perhaps ask on d-d or similar
whether this would be very disruptive.
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.
Ack, I considered whether switching that script to use Dpkg::IPC would
be better, but it does not depend on any other Dpkg module, and it is
intended to be run from the build tree, so your change as-is looks good,
and so I've queued that part too.
And I found another instance in the dpkg-shlibdeps code which I've
also fixed.
I'm attaching the three patches. Thanks!
Regards,
Guillem
From 68bd49c89adec124962a1fd654a488868ac08d00 Mon Sep 17 00:00:00 2001
From: Guillem Jover <
guillem@debian.org>
Date: Thu, 15 Jun 2023 23:49:51 +0200
Subject: [PATCH 1/3] man: Clarify dpkg-architecture -c option
Prompted-by: Paul Wise <
pabs@debian.org>
---
man/dpkg-architecture.pod | 3 +++
1 file changed, 3 insertions(+)
diff --git a/man/dpkg-architecture.pod b/man/dpkg-architecture.pod
index ea963db36..539f700d4 100644
--- a/man/dpkg-architecture.pod
+++ b/man/dpkg-architecture.pod
@@ -93,6 +93,9 @@ Print a similar command to B<--print-set> but to unset all variables.
Execute a I<command-string> in an environment which has all variables
set to the determined value.
+If the I<command-string> contains metacharacters, then it will be invoked +through the system bourne shell.
+
=item B<-L>, B<--list-known>
Print a list of valid architecture names.
--
2.40.1
From be1723de4efcad748e0c7cb0d0cef7c4efd7bcb3 Mon Sep 17 00:00:00 2001
From: Paul Wise <
pabs@debian.org>
Date: Tue, 4 Jul 2023 19:57:57 +0200
Subject: [PATCH 2/3] build: Avoid Perl's exec() falling back to system()
The system/exec functions sometimes execute the command as shell,
passing an indirect object as the first argument avoids that.
The shell usage happe