Hello, I am uploading a 0-day NMU for this bug based on Anthony's patch, after review. The full changeset for the NMU is attached.
Thanks, -- Steve Langasek postmodern programmer
diff -u imms-2.0.1/debian/changelog imms-2.0.1/debian/changelog
--- imms-2.0.1/debian/changelog
+++ imms-2.0.1/debian/changelog
@@ -1,3 +1,14 @@
+imms (2.0.1-3.1) unstable; urgency=high
+
+ * Non-maintainer upload.
+ * High-urgency upload for sarge-targetted RC bugfix
+ * Use pipe() and execlp() directly instead of popen() when calling
+ sox, to avoid arbitrary command execution attacks when acting on
+ files with untrusted names downloaded from the network.
+ Closes: #292777.
+
+ -- Steve Langasek <[EMAIL PROTECTED]> Sat, 5 Feb 2005 04:58:12 -0800
+
imms (2.0.1-3) unstable; urgency=low
* Applied patch from Andreas Jochens to fix a build problem on 64 bit
only in patch2:
unchanged:
--- imms-2.0.1.orig/analyzer/analyzer.cc
+++ imms-2.0.1/analyzer/analyzer.cc
@@ -1,5 +1,7 @@
-#include <stdio.h>
+#include <sys/types.h>
+#include <sys/wait.h>
#include <unistd.h>
+#include <stdio.h>
#include <iostream>
#include <string>
#include <fstream>
@@ -44,17 +46,53 @@
return -4;
}
- ostringstream command;
- command << "nice -n 15 sox \"" << path << "\" -t .raw -w -u -c 1 -r "
- << SAMPLERATE << " -";
-#ifdef DEBUG
- cout << "analyzer: Executing: " << command.str() << endl;
-#endif
- FILE *p = popen(command.str().c_str(), "r");
+ // Do a popen the long way to avoid the shell.
+ int p_fds[2];
+ if (-1 == pipe(p_fds))
+ {
+ cerr << "analyzer: Could not open pipe: " << strerror(errno)
+ << endl;
+ return -2;
+ }
+
+ int child_pid = fork();
+ if (child_pid == -1)
+ {
+ cerr << "analyzer: Could not fork: " << strerror(errno) << endl;
+ return -2;
+ }
+ else if (child_pid == 0)
+ {
+ ostringstream sample_rate;
+ sample_rate << SAMPLERATE;
+
+ if (-1 == dup2(p_fds[1], STDOUT_FILENO))
+ {
+ cerr << "analyzer: Could not dup2: " << strerror(errno)
<< endl;
+ return -2;
+ }
+ if (-1 == nice(15))
+ {
+ cerr << "analyzer: could not renice: " <<
strerror(errno) << endl;
+ return -2;
+ }
+ if (-1 == execlp("sox", "sox", path.c_str(), "-t", ".raw", "-w",
+ "-u", "-c", "1", "-r",
sample_rate.str().c_str(),
+ "-", (char*)NULL))
+ {
+ cerr << "could not exec sox: " << strerror(errno) << endl;
+ return -2;
+ }
+ }
+
+ // The child performed an exec above, so we only get here if we're
+ // the parent.
+ close(p_fds[1]); // otherwise, read wont fail.
+ FILE *p = fdopen(p_fds[0], "r");
if (!p)
{
- cerr << "analyzer: Could not open pipe!" << endl;
+ cerr << "analyzer: Could not fdopen pipe!" << endl;
return -2;
}
@@ -77,10 +115,7 @@
SpectrumAnalyzer analyzer(path);
if (analyzer.is_known())
- {
- cout << "analyzer: Already analyzed - skipping." << endl;
- return 1;
- }
+ throw std::string("analyzer: Already analyzed - skipping.");
while (fread(indata + OVERLAP, sizeof(sample_t), READSIZE, p)
== READSIZE)
@@ -106,7 +141,22 @@
}
catch (std::string &s) { cerr << s << endl; }
- pclose(p);
+ fclose(p);
+ kill(child_pid, SIGKILL); // make sure
+ int sox_status;
+ if (-1 == waitpid(child_pid, &sox_status, 0))
+ cerr << "WARNING: waitpid failed: " << strerror(errno) << endl;
+
+ if (WIFEXITED(sox_status) && 0 != WEXITSTATUS(sox_status))
+ {
+ cerr << "WARNING: sox exited with unclean status "
+ << WEXITSTATUS(sox_status) << ".\n";
+ }
+ else if (WIFSIGNALED(sox_status) && SIGKILL != WTERMSIG(sox_status))
+ {
+ cerr << "WARNING: sox exited on signal " << WTERMSIG(sox_status)
+ << ".\n";
+ }
return 0;
}
signature.asc
Description: Digital signature
