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 Message-ID: <Pine.BSF.4.21.9911262244090.86657-200000@hub.freebsd.org>
index | next in thread | raw e-mail
[-- Attachment #1 --]
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
----
Just remember, as you celebrate Thanksgiving with your family feasts of
turkey, cranberries, stuffing, gravy, mashed potatoes, squash, corn,
cornbread, apples, pickles, dumplings, fish, orangutans, fruitbats,
breakfast cereals, and so forth, to keep in mind the true reason for the
season: The birth of Santa.
[-- Attachment #2 --]
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/11/27 06:41:55
@@ -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");
+ strncpy(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)
+ strncpy(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");
+ strncpy(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);
+ strncpy(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;
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.9911262244090.86657-200000>