summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulian Andres Klode <julian.klode@canonical.com>2024-12-19 17:49:59 +0100
committerJulian Andres Klode <julian.klode@canonical.com>2024-12-19 17:56:49 +0100
commitf6198e45b4cd924f390f977077d96b4545da0c91 (patch)
tree82218db1991c4d4153141b4612cad6bf197632ae
parentb1cb2d37d015579d208bb79f6c4977e20fde12a5 (diff)
Be more strict about which PAGER to execvp() directly
There could be | or ; or other stuff in PAGER that needs special handling, let's encode a list of known-good characters instead. This covers all known pagers. You could also try to execvp() PAGER in its entirety and if it fails with ENOENT fall back to the shell but then you don't get correct error handling if the pager really doesn't exist (in which case we should abort trying to page).
-rw-r--r--apt-private/private-output.cc4
-rwxr-xr-xtest/integration/test-apt-cli-pager11
2 files changed, 14 insertions, 1 deletions
diff --git a/apt-private/private-output.cc b/apt-private/private-output.cc
index e3f795cf3..4480d7382 100644
--- a/apt-private/private-output.cc
+++ b/apt-private/private-output.cc
@@ -165,7 +165,9 @@ bool InitOutputPager()
{
// If our pager name contains a space we need to invoke it in a shell. Boooo!
char *cmd[] = {(char*)"/bin/sh", (char*)"-c", pager.data(), nullptr};
- if (std::none_of(pager.begin(), pager.end(), isspace_ascii))
+ constexpr std::string_view allowed_chars{"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" "+-._=/"};
+ static_assert(allowed_chars.size() == 26 + 26 + 10 + 6);
+ if (pager.find_first_not_of(allowed_chars) == pager.npos)
{
cmd[0] = pager.data();
cmd[1] = nullptr;
diff --git a/test/integration/test-apt-cli-pager b/test/integration/test-apt-cli-pager
index 6d0f88f54..d1f8c3288 100755
--- a/test/integration/test-apt-cli-pager
+++ b/test/integration/test-apt-cli-pager
@@ -17,6 +17,13 @@ setupaptarchive
APTARCHIVE=$(readlink -f ./aptarchive)
+cat >> head3 << EOF
+#!/bin/sh
+exec head -3
+EOF
+
+chmod +x head3
+
for show in info show; do
msgmsg "$show supports pager"
PAGER=cat testsuccessequal "Package: multi
@@ -36,6 +43,10 @@ PAGER="head -3" testsuccessequal "Package: multi
Version: 2.0
Priority: optional" runapt --unbuffer apt $show multi -o TestPager="head -3"
+PAGER="cat|./head3" testsuccessequal "Package: multi
+Version: 2.0
+Priority: optional" runapt --unbuffer apt $show multi -o TestPager="head -3"
+
# Test that we are not blocking
PAGER=more testsuccessequal "Package: multi
Version: 2.0