From 92b2e38dd1334d7f7a30358124c4fad766ca4666 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 7 Sep 2015 19:32:31 +0200 Subject: fix insecure use of /tmp in EDSP solver 'dump' As said in the bugreport, this is hardly a serious problem on a security front, but it was always on the list to have the filename configurable somehow and the stable filename is a problem for parallel executions. Using an environment variable (APT_EDSP_DUMP_FILENAME) for this is more or less the best we can do here as solvers do not get told about our configuration and such. Closes: 795600 --- cmdline/apt-dump-solver.cc | 41 +++++++++++++++------- .../test-external-dependency-solver-protocol | 21 ++++++----- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/cmdline/apt-dump-solver.cc b/cmdline/apt-dump-solver.cc index 4729eac55..2e352931f 100644 --- a/cmdline/apt-dump-solver.cc +++ b/cmdline/apt-dump-solver.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include /*}}}*/ @@ -23,10 +24,11 @@ static bool ShowHelp() { ioprintf(std::cout, "%s %s (%s)\n", PACKAGE, PACKAGE_VERSION, COMMON_ARCH); std::cout << - "Usage: apt-dump-resolver\n" + "Usage: apt-dump-solver\n" "\n" - "apt-dump-resolver is a dummy solver who just dumps its input to the\n" - "file /tmp/dump.edsp and exists with a proper EDSP error.\n" + "apt-dump-solver is a dummy solver who just dumps its input to the\n" + "file specified in the environment variable APT_EDSP_DUMP_FILENAME and\n" + "exists with a proper EDSP error.\n" "\n" " This dump has lost Super Cow Powers.\n"; return true; @@ -39,16 +41,31 @@ int main(int argc,const char *argv[]) /*{{{*/ ShowHelp(); return 0; } - // we really don't need anything - DropPrivileges(); - FILE* input = fdopen(STDIN_FILENO, "r"); - FILE* output = fopen("/tmp/dump.edsp", "w"); - char buffer[400]; - while (fgets(buffer, sizeof(buffer), input) != NULL) - fputs(buffer, output); - fclose(output); - fclose(input); + // we really don't need anything + DropPrivileges(); + char const * const filename = getenv("APT_EDSP_DUMP_FILENAME"); + if (filename == NULL || strlen(filename) == 0) + { + EDSP::WriteError("ERR_NO_FILENAME", "You have to set the environment variable APT_EDSP_DUMP_FILENAME\n" + "to a valid filename to store the dump of EDSP solver input in.\n" + "For example with: export APT_EDSP_DUMP_FILENAME=/tmp/dump.edsp", stdout); + return 0; + } + + unlink(filename); + FileFd input, output; + if (input.OpenDescriptor(STDIN_FILENO, FileFd::ReadOnly) == false || + output.Open(filename, FileFd::WriteOnly | FileFd::Create | FileFd::Exclusive, 0600) == false || + CopyFile(input, output) == false || input.Close() == false || output.Close() == false) + { + std::ostringstream out; + out << "Writing EDSP solver input to file '" << filename << "' failed!\n"; + _error->DumpErrors(out); + EDSP::WriteError("ERR_WRITE_ERROR", out.str(), stdout); + return 0; + } EDSP::WriteError("ERR_JUST_DUMPING", "I am too dumb, i can just dump!\nPlease use one of my friends instead!", stdout); + return 0; } diff --git a/test/integration/test-external-dependency-solver-protocol b/test/integration/test-external-dependency-solver-protocol index 5d5b1c735..6a7a87921 100755 --- a/test/integration/test-external-dependency-solver-protocol +++ b/test/integration/test-external-dependency-solver-protocol @@ -24,7 +24,10 @@ insertpackage 'experimental' 'coolstuff' 'i386,amd64' '3' 'Depends: cool, stuff' setupaptarchive -rm -f /tmp/dump.edsp +testfailure aptget install --solver dump coolstuff -s +testsuccess grep ERR_NO_FILENAME rootdir/tmp/testfailure.output +export APT_EDSP_DUMP_FILENAME="${TMPWORKINGDIRECTORY}/downloaded/dump.edsp" + testfailureequal 'Reading package lists... Building dependency tree... Execute external solver... @@ -34,8 +37,8 @@ I am too dumb, i can just dump! Please use one of my friends instead! E: External solver failed with: I am too dumb, i can just dump!' aptget install --solver dump coolstuff -s -testsuccess test -s /tmp/dump.edsp -rm -f /tmp/dump.edsp +testsuccess test -s "$APT_EDSP_DUMP_FILENAME" +rm -f "$APT_EDSP_DUMP_FILENAME" #FIXME: this should be unstable, but we don't support pinning yet testsuccessequal 'Reading package lists... @@ -58,11 +61,11 @@ Purg cool [1]' aptget purge --solver apt cool -s testsuccess aptget install awesomecoolstuff:i386 -s testsuccess aptget install --solver apt awesomecoolstuff:i386 -s -rm -f /tmp/dump.edsp +rm -f "$APT_EDSP_DUMP_FILENAME" testfailure aptget install --solver dump awesomecoolstuff:i386 -s -testsuccess test -s /tmp/dump.edsp -testequal 'Install: awesomecoolstuff:i386' grep :i386 /tmp/dump.edsp -testempty grep :amd64 /tmp/dump.edsp +testsuccess test -s "$APT_EDSP_DUMP_FILENAME" +testequal 'Install: awesomecoolstuff:i386' grep :i386 "$APT_EDSP_DUMP_FILENAME" +testempty grep -e ':amd64' -e 'Architecture: any' "$APT_EDSP_DUMP_FILENAME" testsuccess aptget dist-upgrade -s testsuccess aptget dist-upgrade -s --solver apt @@ -76,14 +79,14 @@ testsuccess grep 'ERR_UNSOLVABLE' rootdir/tmp/testfailure.output configarchitecture 'armel' msgtest 'Test direct calling is okay for' 'apt-internal-solver' -cat /tmp/dump.edsp | aptinternalsolver -q=0 > solver.result 2>&1 || true +cat "$APT_EDSP_DUMP_FILENAME" | aptinternalsolver -q=0 > solver.result 2>&1 || true if [ "$(tail -n2 solver.result | head -n1 )" = "Message: Done" ]; then msgpass else cat solver.result msgfail fi -rm -f /tmp/dump.edsp +rm -f "$APT_EDSP_DUMP_FILENAME" testfailure aptget install --solver apt awesomecoolstuff:i386 -s -- cgit v1.2.3