Date: Sat, 4 Dec 1999 19:41:04 -0800 (PST) From: Kris Kennaway <kris@hub.freebsd.org> To: audit@freebsd.org Subject: ctm_rmail holes (fwd) Message-ID: <Pine.BSF.4.21.9912041930430.3023-100000@hub.freebsd.org>
next in thread | raw e-mail | index | archive | help
I haven't heard any responses about this one yet..can someone give it the
twice-over? I'd kind of like to close this remote buffer overflow :-)
Kris
---------- Forwarded message ----------
Date: Fri, 26 Nov 1999 22:56:42 -0800 (PST)
From: Kris Kennaway <kris@hub.freebsd.org>
To: audit@freebsd.org
Subject: ctm_rmail holes
There are a couple of buffer overflows in ctm_rmail (part of ctm which
automatically decodes and applies deltas received via email) which look
like they could be exploitable by sending a malformed email. OpenBSD fixed
these, but for some reason Theo backed them out a few months ago during a
sync with our code.
Of course, a larger issue with CTM is that it looks like anyone can insert
their code into your source tree just by sending a delta to you, because
it does no authentication whatsoever of the contents except that it
applies cleanly :-(
The attached patch syncs with the OpenBSD security changes, and I've also
fixed a lock file race, and some command-line buffer overflows which
weren't likely to be security problems. However I can't test this, because
I don't have my machine set up to use CTM.
Comments, anyone?
Kris
Index: ctm_rmail.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/ctm/ctm_rmail/ctm_rmail.c,v
retrieving revision 1.14
diff -u -r1.14 ctm_rmail.c
--- ctm_rmail.c 1998/07/27 22:26:25 1.14
+++ ctm_rmail.c 1999/12/05 03:34:07
@@ -10,6 +10,11 @@
* Maybe you should write some free software too.
*/
+#ifndef lint
+static const char rcsid[] =
+"$FreeBSD$";
+#endif /* not lint */
+
#include <stdio.h>
#include <stdlib.h>
#include <strings.h>
@@ -35,7 +40,8 @@
void apply_complete(void);
int read_piece(char *input_file);
int combine_if_complete(char *delta, int pce, int npieces);
-int combine(char *delta, int npieces, char *dname, char *pname, char *tname);
+int combine(char *delta, int npieces, char *dname, char *pname, char *tname,
+ int len);
int decode_line(char *line, char *out_buf);
int lock_file(char *name);
@@ -118,14 +124,14 @@
/*
* Construct the file name of a piece of a delta.
*/
-#define mk_piece_name(fn,d,p,n) \
- sprintf((fn), "%s/%s+%03d-%03d", piece_dir, (d), (p), (n))
+#define mk_piece_name(fn,l,d,p,n) \
+ snprintf((fn), l, "%s/%s+%03d-%03d", piece_dir, (d), (p), (n))
/*
* Construct the file name of an assembled delta.
*/
-#define mk_delta_name(fn,d) \
- sprintf((fn), "%s/%s", delta_dir, (d))
+#define mk_delta_name(fn,l,d) \
+ snprintf((fn), l, "%s/%s", delta_dir, (d))
/*
* If the next required delta is now present, let ctm lunch on it and any
@@ -149,22 +155,22 @@
* Grab a lock on the ctm mutex file so that we can be sure we are
* working alone, not fighting another ctm_rmail!
*/
- strcpy(fname, delta_dir);
- strcat(fname, "/.mutex_apply");
+ strlcpy(fname, delta_dir, sizeof(fname));
+ strlcat(fname, "/.mutex_apply", sizeof(fname));
if ((lfd = lock_file(fname)) < 0)
return;
/*
* Find out which delta ctm needs next.
*/
- sprintf(fname, "%s/%s", base_dir, CTM_STATUS);
+ snprintf(fname, sizeof(fname), "%s/%s", base_dir, CTM_STATUS);
if ((fp = fopen(fname, "r")) == NULL)
{
close(lfd);
return;
}
- i = fscanf(fp, "%s %d %c", class, &dn, junk);
+ i = fscanf(fp, "%19s %d %c", class, &dn, junk);
fclose(fp);
if (i != 2)
{
@@ -178,7 +184,7 @@
here[0] = '\0';
if (delta_dir[0] != '/')
{
- getcwd(here, sizeof(here)-1);
+ getcwd(here, sizeof(here)-2);
i = strlen(here) - 1;
if (i >= 0 && here[i] != '/')
{
@@ -192,13 +198,13 @@
*/
for (;;)
{
- sprintf(delta, "%s.%04d.gz", class, ++dn);
- mk_delta_name(fname, delta);
+ snprintf(delta, sizeof(delta), "%s.%04d.gz", class, ++dn);
+ mk_delta_name(fname, sizeof(fname), delta);
if (stat(fname, &sb) < 0)
break;
- sprintf(buf, "(cd %s && ctm %s%s%s%s) 2>&1", base_dir,
+ snprintf(buf, sizeof(buf), "(cd %s && ctm %s%s%s%s) 2>&1", base_dir,
set_time ? "-u " : "",
apply_verbose ? "-v " : "", here, fname);
if ((ctm = popen(buf, "r")) == NULL)
@@ -294,7 +300,7 @@
{
char *s;
- if (sscanf(line, "CTM_MAIL BEGIN %s %d %d %c",
+ if (sscanf(line, "CTM_MAIL BEGIN %29s %d %d %c",
delta, &pce, &npieces, junk) != 3)
continue;
@@ -302,17 +308,17 @@
*s = '_';
got_one++;
- strcpy(tname, piece_dir);
- strcat(tname, "/p.XXXXXX");
- if ((ofd = mkstemp(tname)) < 0)
+ strlcpy(tname, piece_dir, sizeof(tname));
+ strlcat(tname, "/p.XXXXXXXXXX", sizeof(tname));
+ if ((ofd = mkstemp(tname)) == -1 ||
+ (ofp = fdopen(ofd, "w")) == NULL)
{
- err("*mkstemp: '%s'", tname);
- status++;
- continue;
- }
- if ((ofp = fdopen(ofd, "w")) == NULL)
- {
- err("cannot open '%s' for writing", tname);
+ if (ofd != -1) {
+ err("cannot open '%s' for writing", tname);
+ close(ofd);
+ }
+ else
+ err("*mkstemp: '%s'", tname);
status++;
continue;
}
@@ -349,7 +355,7 @@
continue;
}
- mk_piece_name(pname, delta, pce, npieces);
+ mk_piece_name(pname, sizeof(pname), delta, pce, npieces);
if (rename(tname, pname) < 0)
{
err("*rename: '%s' to '%s'", tname, pname);
@@ -438,8 +444,8 @@
*/
if (npieces == 1)
{
- mk_delta_name(dname, delta);
- mk_piece_name(pname, delta, 1, 1);
+ mk_delta_name(dname, sizeof(dname), delta);
+ mk_piece_name(pname, sizeof(pname), delta, 1, 1);
if (rename(pname, dname) == 0)
{
err("%s complete", delta);
@@ -451,8 +457,8 @@
* Grab a lock on the reassembly mutex file so that we can be sure we are
* working alone, not fighting another ctm_rmail!
*/
- strcpy(tname, delta_dir);
- strcat(tname, "/.mutex_build");
+ strlcpy(tname, delta_dir, sizeof(tname));
+ strlcat(tname, "/.mutex_build", sizeof(tname));
if ((lfd = lock_file(tname)) < 0)
return 0;
@@ -465,7 +471,7 @@
{
if (i == pce)
continue;
- mk_piece_name(pname, delta, i, npieces);
+ mk_piece_name(pname, sizeof(pname), delta, i, npieces);
if (stat(pname, &sb) < 0)
{
close(lfd);
@@ -477,7 +483,7 @@
* Stick them together. Let combine() use our file name buffers, since
* we're such good buddies. :-)
*/
- e = combine(delta, npieces, dname, pname, tname);
+ e = combine(delta, npieces, dname, pname, tname, sizeof(tname));
close(lfd);
return e;
}
@@ -490,23 +496,24 @@
* happened to by lying around in the calling routine. Waste not, want not!
*/
int
-combine(char *delta, int npieces, char *dname, char *pname, char *tname)
+combine(char *delta, int npieces, char *dname, char *pname, char *tname, int len)
{
FILE *dfp, *pfp;
int dfd;
int i, n, e;
char buf[BUFSIZ];
- strcpy(tname, delta_dir);
- strcat(tname, "/d.XXXXXX");
- if ((dfd = mkstemp(tname)) < 0)
- {
- err("*mkstemp: '%s'", tname);
- return 0;
- }
- if ((dfp = fdopen(dfd, "w")) == NULL)
- {
- err("cannot open '%s' for writing", tname);
+ strlcpy(tname, delta_dir, len);
+ strlcat(tname, "/d.XXXXXXXXXX", len);
+ if ((dfd = mkstemp(tname)) == -1 ||
+ (dfp = fdopen(dfd, "w")) == NULL)
+ {
+ if (dfd != -1) {
+ close(dfd);
+ err("cannot open '%s' for writing", tname);
+ }
+ else
+ err("*mktemp: '%s'", tname);
return 0;
}
@@ -515,7 +522,7 @@
*/
for (i = 1; i <= npieces; i++)
{
- mk_piece_name(pname, delta, i, npieces);
+ mk_piece_name(pname, len, delta, i, npieces);
if ((pfp = fopen(pname, "r")) == NULL)
{
err("cannot open '%s' for reading", pname);
@@ -545,7 +552,7 @@
return 0;
}
- mk_delta_name(dname, delta);
+ mk_delta_name(dname, len, delta);
if (rename(tname, dname) < 0)
{
err("*rename: '%s' to '%s'", tname, dname);
@@ -558,7 +565,7 @@
*/
for (i = 1; i <= npieces; i++)
{
- mk_piece_name(pname, delta, i, npieces);
+ mk_piece_name(pname, len, delta, i, npieces);
if (unlink(pname) < 0)
err("*unlink: '%s'", pname);
}
@@ -648,15 +655,9 @@
{
int lfd;
- if ((lfd = open(name, O_WRONLY|O_CREAT, 0600)) < 0)
+ if ((lfd = open(name, O_WRONLY|O_CREAT|O_EXLOCK, 0600)) < 0)
{
err("*open: '%s'", name);
- return -1;
- }
- if (flock(lfd, LOCK_EX) < 0)
- {
- close(lfd);
- err("*flock: '%s'", name);
return -1;
}
return lfd;
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.9912041930430.3023-100000>