CUPS 1.1.22 lppasswd ignores write

[michaelrsweet]

Version: 1.1.22
CUPS.org User: d.j.bernstein

Bartlomiej Sieka, a student in my Fall 2004 UNIX Security Holes course,
has discovered several security problems in how lppasswd, version 1.1.22
(current), edits /usr/local/etc/cups/passwd. I'm publishing this notice,
but all the discovery credits should be assigned to Sieka.

First, lppasswd blithely ignores write errors in fputs(line,outfile) at
lines 311 and 315 of lppasswd.c, and in fprintf(...) at line 346. An
attacker who fills up the disk at the right moment can arrange for
/usr/local/etc/cups/passwd to be truncated.

Second, if lppasswd bumps into a file-size resource limit while writing
passwd.new, it leaves passwd.new in place, disabling all subsequent
invocations of lppasswd. Any local user can thus disable lppasswd by
running the attached program 63.c.

Third, line 306 of lppasswd.c prints an error message to stderr but
does not exit. This is not a problem on systems that ensure that file
descriptors 0, 1, and 2 are open for setuid programs, but it is a
problem on other systems; lppasswd does not check that passwd.new is
different from stderr, so it ends up writing a user-controlled error
message to passwd if the user closes file descriptor 2.

---D. J. Bernstein, Associate Professor, Department of Mathematics,
Statistics, and Computer Science, University of Illinois at Chicago

[michaelrsweet]

CUPS.org User: twaugh.redhat

How about the attached patch?

[michaelrsweet]

CUPS.org User: mike

SIGXFSZ isn't portable...

Anyways, I think simple checks on the open, write, and close calls should be enough to fix things - new patch coming up...

[michaelrsweet]

CUPS.org User: mike

OK, try str1023esp.patch, which checks that fd's 0, 1, and 2 are open, ignores many signals (including SIGXFSZ if it is defined), and does a lot better error handling along the way to ensure that passwd.new is never left behind.

[michaelrsweet]

CUPS.org User: twaugh.redhat

If it just ignores the signal won't it truncate the password file?

[michaelrsweet]

CUPS.org User: mike

No, by ignoring the signals we prevent interruption and possible truncation. If we hit the filesize limit, we get an error back from fputs() and abort cleanly.

An alternate implementation would be to provide a signal handler which removes the passwd.new file, however you'd still have the open files and renaming to deal with...

[michaelrsweet]

"str1023.patch":

[michaelrsweet]

"cups-str1023.patch":

--- cups-1.1.22/systemv/lppasswd.c.str1023 2004-02-25 20:14:54.000000000 +0000
+++ cups-1.1.22/systemv/lppasswd.c 2004-12-16 13:49:25.301361501 +0000
@@ -39,6 +39,7 @@
#include <grp.h>
#include <sys/types.h>
#include <sys/stat.h>
+#include <signal.h>

#include <cups/cups.h>
#include <cups/md5.h>
@@ -60,6 +61,12 @@

static void usage(FILE *fp);

+static int must_exit;
+void handle_xfsz (int signum)
+{

• must_exit = 1;
  +}

/*

• 'main()' - Add, change, or delete passwords from the MD5 password file.
  @@ -90,7 +97,15 @@
  oldpass; / old password /
  int flag; / Password check flags... /
  int fd; / Password file descriptor */
• struct sigaction sa;
• memset (&sa, 0, sizeof (sa));
• sa.sa_handler = handle_xfsz;
• if (sigaction (SIGXFSZ, &sa, NULL))
• {
• fprintf (stderr, "lppasswd: can't set signal handler\n");
• return (1);
• }

/*

• Find the server directory...
  @@ -308,11 +323,25 @@
  strcmp(groupname, groupline) == 0)
  break;

•  fputs(line, outfile);
  

•  if (fputs(line, outfile) < 0 || must_exit)
  

•  {
  

• fputs("lppasswd: write failed!\n", stderr);

• fclose(outfile);

• unlink(passwdnew);

• return (1);

•  }
  

  }

  while (fgets(line, sizeof(line), infile) != NULL)

•  fputs(line, outfile);
  

• {

•  if (fputs(line, outfile) < 0 || must_exit)
  

•  {
  

• fputs("lppasswd: write failed!\n", stderr);

• fclose(outfile);

• unlink(passwdnew);

• return (1);

•  }
  

• }
  }
  else
  {
  @@ -324,8 +353,15 @@
  if (op == CHANGE &&
  (strcmp(username, userline) != 0 ||
  strcmp(groupname, groupline) != 0))

• {
  fprintf(stderr, "lppasswd: user "%s" and group "%s" do not exist.\n",
  username, groupname);

• if (infile)

•  fclose(infile);
  

• fclose(outfile);

• unlink(passwdnew);

• return (1);

• }
  else if (op != DELETE)
  {
  if (oldpass &&
  @@ -343,8 +379,14 @@
  return (1);
  }

• fprintf(outfile, "%s:%s:%s\n", username, groupname,

•        httpMD5(username, "CUPS", newpass, md5new));
  

• if (fprintf(outfile, "%s:%s:%s\n", username, groupname,

•   httpMD5(username, "CUPS", newpass, md5new)) < 0 || must_exit)
  

• {

• fputs("lppasswd: write failed!\n", stderr);

• fclose(outfile);

• unlink(passwdnew);

• return (1);

• }
  }

/*
@@ -354,7 +396,12 @@
if (infile)
fclose(infile);

• fclose(outfile);
• if (fclose(outfile) || must_exit)
• {
• fputs ("lppasswd: write failed!\n", stderr);
• unlink(passwdnew);
• return(1);
• }

/*

• Save old passwd file

[michaelrsweet]

"str1023esp.patch":

Index: lppasswd.c

RCS file: /development/cvs/cups/systemv/lppasswd.c,v
retrieving revision 1.16
diff -u -r1.16 lppasswd.c
--- lppasswd.c 25 Feb 2004 20:14:54 -0000 1.16
+++ lppasswd.c 16 Dec 2004 19:28:18 -0000
@@ -44,6 +44,11 @@
#include <cups/md5.h>
#include <cups/string.h>

+#ifndef WIN32
+# include <unistd.h>
+# include <signal.h>
+#endif /* !WIN32 */
+

/*

• Operations...
  @@ -90,7 +95,27 @@
  oldpass; / old password /
  int flag; / Password check flags... /
  int fd; / Password file descriptor */
  • int error; /* Write error */
    +#if defined(HAVE_SIGACTION) && !defined(HAVE_SIGSET)
  • struct sigaction action; /* Signal action /
    +#endif / HAVE_SIGACTION && !HAVE_SIGSET*/
  • /*
  • * Check to see if stdin, stdout, and stderr are still open...
  • */
  • if (fcntl(0, F_GETFD, &i) ||

  •  fcntl(1, F_GETFD, &i) ||
    

  •  fcntl(2, F_GETFD, &i))
    

  • {
  • /*
  • * No, return exit status 2 and don't try to send any output since
  • * someone is trying to bypass the security on the server.
  • */
• return (2);
• }

/*

• Find the server directory...
  @@ -253,6 +278,40 @@
  }

/*

• * Ignore SIGHUP, SIGINT, SIGTERM, and SIGXFSZ (if defined) for the
• * remainder of the time so that we won't end up with bogus password
• * files...
• */

+#ifndef WIN32
+# if defined(HAVE_SIGSET)

• sigset(SIGHUP, SIG_IGN);
• sigset(SIGINT, SIG_IGN);
• sigset(SIGTERM, SIG_IGN);
  +# ifdef SIGXFSZ
• sigset(SIGXFSZ, SIG_IGN);
  +# endif /* SIGXFSZ */
  +# elif defined(HAVE_SIGACTION)
• memset(&action, 0, sizeof(action));
• action.sa_handler = SIG_IGN;

• sigaction(SIGHUP, &action, NULL);
• sigaction(SIGINT, &action, NULL);
• sigaction(SIGTERM, &action, NULL);
  +# ifdef SIGXFSZ
• sigaction(SIGXFSZ, &action, NULL);
  +# endif /* SIGXFSZ */
  +# else
• signal(SIGHUP, SIG_IGN);
• signal(SIGINT, SIG_IGN);
• signal(SIGTERM, SIG_IGN);
  +# ifdef SIGXFSZ
• signal(SIGXFSZ, SIG_IGN);
  +# endif /* SIGXFSZ /
  +# endif
  +#endif / !WIN32 */

• /*
  • Open the output file.
    */

@@ -275,6 +334,8 @@
return (1);
}

• setbuf(outfile, NULL);

/*

• Open the existing password file and create a new one...
  */
  @@ -282,7 +343,7 @@
  infile = fopen(passwdmd5, "r");
  if (infile == NULL && errno != ENOENT && op != ADD)
  {
• fputs("lppasswd: No password file to change or delete from!\n", stderr);
• perror("lppasswd: Unable to open password file");

fclose(outfile);

@@ -297,6 +358,11 @@

• username:group:MD5-sum
  */
• error = 0;
• userline[0] = '\0';
• groupline[0] = '\0';
• md5line[0] = '\0';

if (infile)
{
while (fgets(line, sizeof(line), infile) != NULL)
@@ -308,60 +374,87 @@
strcmp(groupname, groupline) == 0)
break;

•  fputs(line, outfile);
  

•  if (fputs(line, outfile) == EOF)
  

•  {
  

•    perror("lppasswd: Unable to write to password file");
  

•    error = 1;
  

• break;

•  }
  

  }

• while (fgets(line, sizeof(line), infile) != NULL)

•  fputs(line, outfile);
  

• }

• else

• {

• userline[0] = '\0';

• groupline[0] = '\0';

• md5line[0] = '\0';

• if (!error)

• {

•  while (fgets(line, sizeof(line), infile) != NULL)
  

• if (fputs(line, outfile) == EOF)

• {

•      perror("lppasswd: Unable to write to password file");
  

• error = 1;
  

• break;
  

• }

• }
  }

if (op == CHANGE &&

•  (strcmp(username, userline) != 0 ||
  

•   strcmp(groupname, groupline) != 0))
  

•  (strcmp(username, userline) || strcmp(groupname, groupline)))
  

• {
  fprintf(stderr, "lppasswd: user "%s" and group "%s" do not exist.\n",
  username, groupname);
• error = 1;
• }
  else if (op != DELETE)
  {
  if (oldpass &&
  strcmp(httpMD5(username, "CUPS", oldpass, md5new), md5line) != 0)
  {
  fputs("lppasswd: Sorry, password doesn't match!\n", stderr);

•  if (infile)
  

- fclose(infile);

- fclose(outfile);

- unlink(passwdnew);

•  return (1);
  

•  error = 1;
  

• }

• else

• {

•  snprintf(line, sizeof(line), "%s:%s:%s\n", username, groupname,
  

•           httpMD5(username, "CUPS", newpass, md5new));
  

•  if (fputs(line, outfile) == EOF)
  

•  {
  

•    perror("lppasswd: Unable to write to password file");
  

•    error = 1;
  

•  }
  

  }

• fprintf(outfile, "%s:%s:%s\n", username, groupname,

•        httpMD5(username, "CUPS", newpass, md5new));
  

  }

  /*

• * Close the files and remove the old password file...

• • Close the files...
    */

  if (infile)
  fclose(infile);

• fclose(outfile);

• if (fclose(outfile) == EOF)

• error = 1;

• /*
• * Error out gracefully as needed...
• */

• if (error)
• {
• fputs("lppasswd: Password file not updated!\n", stderr);
• unlink(passwdnew);

• return (1);
• }

/*

• Save old passwd file
  */

unlink(passwdold);

• link(passwdmd5, passwdold);
• if (link(passwdmd5, passwdold))
• {
• perror("lppasswd: failed to backup old password file");
• unlink(passwdnew);
• return (1);
• }

/*

• Install new password file
  @@ -369,7 +462,7 @@

if (rename(passwdnew, passwdmd5) < 0)
{

• perror("lppasswd: failed to rename passwd file");
• perror("lppasswd: failed to rename password file");
  unlink(passwdnew);
  return (1);
  }