Browse Source

s-s-d: Fix off-by-one stack buffer overrun on GNU/Linux and GNU/kFreeBSD

This might happen if the executable pathname is longer than
_POSIX_PATH_MAX. Although this should not have security implications
as the buffer is surrounded by two arrays (so those catch accesses
even if the stack grows up or down), and we are compiling with
-fstack-protector anyway.

We just need to always leave room for the final NUL character.

Warned-by: coverity
Guillem Jover 12 years ago
parent
commit
00e2aadcdc
2 changed files with 7 additions and 2 deletions
  1. 5 0
      debian/changelog
  2. 2 2
      utils/start-stop-daemon.c

+ 5 - 0
debian/changelog

@@ -11,6 +11,11 @@ dpkg (1.17.9) UNRELEASED; urgency=low
   * Fix memory leak in unused Keybindings screen in dselect.
   * Do not leak color string on «dselect --color».
   * Fix memory leaks when parsing alternatives.
+  * Fix off-by-one stack buffer overrun in start-stop-daemon on GNU/Linux and
+    GNU/kFreeBSD if the executable pathname is longer than _POSIX_PATH_MAX.
+    Although this should not have security implications as the buffer is
+    surrounded by two arrays (so those catch accesses even if the stack
+    grows up or down), and we are compiling with -fstack-protector anyway.
 
   [ Updated scripts translations ]
   * French (Steve Petruzzello). Closes: #746350

+ 2 - 2
utils/start-stop-daemon.c

@@ -1165,13 +1165,13 @@ static bool
 pid_is_exec(pid_t pid, const struct stat *esb)
 {
 	char lname[32];
-	char lcontents[_POSIX_PATH_MAX];
+	char lcontents[_POSIX_PATH_MAX + 1];
 	const char deleted[] = " (deleted)";
 	int nread;
 	struct stat sb;
 
 	sprintf(lname, "/proc/%d/exe", pid);
-	nread = readlink(lname, lcontents, sizeof(lcontents));
+	nread = readlink(lname, lcontents, sizeof(lcontents) - 1);
 	if (nread == -1)
 		return false;