This is a general bounded-buffer patch for sysinstall plus it fixes
PR misc/18466 (limited 'device name' size screws up FTP installs).
It does not fix all the potential buffer overflows -- there are still
a lot of strcpy's, but it gets half way there with the introduction
and use of safe_asprintf().
My recommendation is that (eventually) the entire program use asprintf
rather then statically-bounded buffers to hold things.
I don't have time to test this extensively so if the maintainer of
sysinstall wants it (I believe that's you, Jordan!), you'll need to
review it and perhaps test it a little then give me the commit goahead.
Note that simply increasing the DEV_NAME_MAX as a quick solution is
not safe, there are a couple of places where an unbounded sprintf()
was being used with the device name as an argument.
-Matt
Index: anonFTP.c
===
RCS file: /home/ncvs/src/release/sysinstall/anonFTP.c,v
retrieving revision 1.29
diff -u -r1.29 anonFTP.c
--- anonFTP.c 2000/01/25 19:16:31 1.29
+++ anonFTP.c 2000/05/14 17:44:11
@@ -168,7 +168,7 @@
return DITEM_SUCCESS; /* succeeds if already exists */
}
-sprintf(pwline, "%s:*:%s:%d::0:0:%s:%s:/nonexistent\n", FTP_NAME, tconf.uid, gid,
tconf.comment, tconf.homedir);
+snprintf(pwline, sizeof(pwline), "%s:*:%s:%d::0:0:%s:%s:/nonexistent\n",
+FTP_NAME, tconf.uid, gid, tconf.comment, tconf.homedir);
fptr = fopen(_PATH_MASTERPASSWD,"a");
if (! fptr) {
@@ -207,7 +207,7 @@
ANONFTP_DIALOG_HEIGHT - 11, ANONFTP_DIALOG_WIDTH - 17,
dialog_attr, border_attr);
wattrset(ds_win, dialog_attr);
-sprintf(title, " Path Configuration ");
+snprintf(title, sizeof(title), " Path Configuration ");
mvwaddstr(ds_win, ANONFTP_DIALOG_Y + 7, ANONFTP_DIALOG_X + 22, title);
/** Initialize the config Data Structure **/
@@ -217,7 +217,7 @@
SAFE_STRCPY(tconf.upload, FTP_UPLOAD);
SAFE_STRCPY(tconf.comment, FTP_COMMENT);
SAFE_STRCPY(tconf.homedir, FTP_HOMEDIR);
-sprintf(tconf.uid, "%d", FTP_UID);
+snprintf(tconf.uid, sizeof(tconf.uid), "%d", FTP_UID);
/* Some more initialisation before we go into the main input loop */
obj = initLayoutDialog(ds_win, layout, ANONFTP_DIALOG_X, ANONFTP_DIALOG_Y, max);
@@ -250,7 +250,7 @@
/*** Use defaults for any invalid values ***/
if (atoi(tconf.uid) = 0)
- sprintf(tconf.uid, "%d", FTP_UID);
+ snprintf(tconf.uid, sizeof(tconf.uid), "%d", FTP_UID);
if (!tconf.group[0])
SAFE_STRCPY(tconf.group, FTP_GROUP);
@@ -296,7 +296,7 @@
if (!msgYesNo("Create a welcome message file for anonymous FTP users?")) {
char cmd[256];
vsystem("echo Your welcome message here. %s/etc/%s", tconf.homedir,
MOTD_FILE);
- sprintf(cmd, "%s %s/etc/%s", variable_get(VAR_EDITOR), tconf.homedir,
MOTD_FILE);
+ snprintf(cmd, sizeof(cmd), "%s %s/etc/%s", variable_get(VAR_EDITOR),
+tconf.homedir, MOTD_FILE);
if (!systemExecute(cmd))
i = DITEM_SUCCESS;
else
Index: config.c
===
RCS file: /home/ncvs/src/release/sysinstall/config.c,v
retrieving revision 1.156.2.1
diff -u -r1.156.2.1 config.c
--- config.c2000/03/30 08:12:02 1.156.2.1
+++ config.c2000/05/14 17:46:38
@@ -238,11 +238,11 @@
for (i = 0; i cnt; i++) {
char cdname[10];
- sprintf(cdname, "/cdrom%s", i ? itoa(i) : "");
+ snprintf(cdname, sizeof(cdname), "/cdrom%s", i ? itoa(i) : "");
if (Mkdir(cdname))
msgConfirm("Unable to make mount point for: %s", cdname);
else
- fprintf(fstab, "/dev/%s\t\t%s\tcd9660\tro,noauto\t0\t0\n", devs[i]-name,
cdname);
+ fprintf(fstab, "/dev/%s\t\t%s\tcd9660\tro,noauto\t0\t0\n", devs[i]-pname,
+cdname);
}
/* And finally, a /proc. */
@@ -364,6 +364,8 @@
}
}
+#define URMSIZE21
+
/* Set up the make.conf file */
void
configMake_conf(char *config)
@@ -373,8 +375,8 @@
FILE *fp;
if (!file_readable(config)) {
- char *line = malloc(21);
- sprintf(line, "USA_RESIDENT=%s\n", USAResident ? "YES" : "NO");
+ char *line = malloc(URMSIZE);
+ snprintf(line, URMSIZE, "USA_RESIDENT=%s\n", USAResident ? "YES" : "NO");
lines[0] = line;
nlines = 1;
}
@@ -386,7 +388,7 @@
if (!strncmp(lines[i], "USA_RESIDENT", 12)) {
free(lines[i]);
lines[i] = malloc(21); /* big enough */
- sprintf(lines[i], "USA_RESIDENT=%s\n", USAResident ? "YES" : "NO");
+ snprintf(lines[i], URMSIZE, "USA_RESIDENT=%s\n", USAResident ? "YES" :
+"NO");
}
}
}
@@ -865,7 +867,7 @@
