git: a78807a2fa0e - stable/13 - bsdinstall partedit: Avoid potential buffer overflow in newfs_command
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 05 Jan 2024 00:23:07 UTC
The branch stable/13 has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=a78807a2fa0e2d81954e984883477c9b567b8617 commit a78807a2fa0e2d81954e984883477c9b567b8617 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2023-10-16 23:25:03 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2024-01-04 23:51:34 +0000 bsdinstall partedit: Avoid potential buffer overflow in newfs_command Allocate the buffer holding the newfs command string dynamically (building the string via open_memstream) rather than storing the command into a caller-supplied buffer of unknown length. Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D42237 (cherry picked from commit ae2fc74fe76ca8b89c5ef0081ef3f4008f83de41) --- usr.sbin/bsdinstall/partedit/gpart_ops.c | 70 +++++++++++++++++++------------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/usr.sbin/bsdinstall/partedit/gpart_ops.c b/usr.sbin/bsdinstall/partedit/gpart_ops.c index 368ef9b14727..331cbc4ffba6 100644 --- a/usr.sbin/bsdinstall/partedit/gpart_ops.c +++ b/usr.sbin/bsdinstall/partedit/gpart_ops.c @@ -78,9 +78,15 @@ scheme_supports_labels(const char *scheme) return (0); } -static void -newfs_command(const char *fstype, char *command, int use_default) +static char * +newfs_command(const char *fstype, int use_default) { + FILE *fp; + char *buf; + size_t len; + + fp = open_memstream(&buf, &len); + if (strcmp(fstype, "freebsd-ufs") == 0) { int i; DIALOG_LISTITEM items[] = { @@ -103,21 +109,21 @@ newfs_command(const char *fstype, char *command, int use_default) nitems(items), items, NULL, FLAG_CHECK, &i); if (choice == 1) /* Cancel */ - return; + goto out; } - strcpy(command, "newfs "); + fputs("newfs ", fp); for (i = 0; i < (int)nitems(items); i++) { if (items[i].state == 0) continue; if (strcmp(items[i].name, "UFS1") == 0) - strcat(command, "-O1 "); + fputs("-O1 ", fp); else if (strcmp(items[i].name, "SU") == 0) - strcat(command, "-U "); + fputs("-U ", fp); else if (strcmp(items[i].name, "SUJ") == 0) - strcat(command, "-j "); + fputs("-j ", fp); else if (strcmp(items[i].name, "TRIM") == 0) - strcat(command, "-t "); + fputs("-t ", fp); } } else if (strcmp(fstype, "freebsd-zfs") == 0) { int i; @@ -141,30 +147,31 @@ newfs_command(const char *fstype, char *command, int use_default) nitems(items), items, NULL, FLAG_CHECK, &i); if (choice == 1) /* Cancel */ - return; + goto out; } - strcpy(command, "zpool create -f -m none "); + fputs("zpool create -f -m none ", fp); if (getenv("BSDINSTALL_TMPBOOT") != NULL) { char zfsboot_path[MAXPATHLEN]; + snprintf(zfsboot_path, sizeof(zfsboot_path), "%s/zfs", getenv("BSDINSTALL_TMPBOOT")); mkdir(zfsboot_path, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH); - sprintf(command, "%s -o cachefile=%s/zpool.cache ", - command, zfsboot_path); + fprintf(fp, " -o cachefile=%s/zpool.cache ", + zfsboot_path); } for (i = 0; i < (int)nitems(items); i++) { if (items[i].state == 0) continue; if (strcmp(items[i].name, "fletcher4") == 0) - strcat(command, "-O checksum=fletcher4 "); + fputs("-O checksum=fletcher4 ", fp); else if (strcmp(items[i].name, "fletcher2") == 0) - strcat(command, "-O checksum=fletcher2 "); + fputs("-O checksum=fletcher2 ", fp); else if (strcmp(items[i].name, "sha256") == 0) - strcat(command, "-O checksum=sha256 "); + fputs("-O checksum=sha256 ", fp); else if (strcmp(items[i].name, "atime") == 0) - strcat(command, "-O atime=off "); + fputs("-O atime=off ", fp); } } else if (strcmp(fstype, "fat32") == 0 || strcmp(fstype, "efi") == 0 || strcmp(fstype, "ms-basic-data") == 0) { @@ -184,26 +191,29 @@ newfs_command(const char *fstype, char *command, int use_default) nitems(items), items, NULL, FLAG_RADIO, &i); if (choice == 1) /* Cancel */ - return; + goto out; } - strcpy(command, "newfs_msdos "); + fputs("newfs_msdos ", fp); for (i = 0; i < (int)nitems(items); i++) { if (items[i].state == 0) continue; if (strcmp(items[i].name, "FAT32") == 0) - strcat(command, "-F 32 -c 1"); + fputs("-F 32 -c 1", fp); else if (strcmp(items[i].name, "FAT16") == 0) - strcat(command, "-F 16 "); + fputs("-F 16 ", fp); else if (strcmp(items[i].name, "FAT12") == 0) - strcat(command, "-F 12 "); + fputs("-F 12 ", fp); } } else { if (!use_default) dialog_msgbox("Error", "No configurable options exist " "for this filesystem.", 0, 0, TRUE); - command[0] = '\0'; } + +out: + fclose(fp); + return (buf); } const char * @@ -509,7 +519,7 @@ gpart_edit(struct gprovider *pp) const char *errstr, *oldtype, *scheme; struct partition_metadata *md; char sizestr[32]; - char newfs[255]; + char *newfs; intmax_t idx; int hadlabel, choice, junk, nitems; unsigned i; @@ -650,10 +660,11 @@ editpart: } gctl_free(r); - newfs_command(items[0].text, newfs, 1); + newfs = newfs_command(items[0].text, 1); set_default_part_metadata(pp->lg_name, scheme, items[0].text, items[2].text, (strcmp(oldtype, items[0].text) != 0) ? newfs : NULL); + free(newfs); endedit: if (strcmp(oldtype, items[0].text) != 0 && cp != NULL) @@ -981,7 +992,7 @@ gpart_create(struct gprovider *pp, const char *default_type, struct ggeom *geom; const char *errstr, *scheme; char sizestr[32], startstr[32], output[64], *newpartname; - char newfs[255], options_fstype[64]; + char *newfs, options_fstype[64]; intmax_t maxsize, size, sector, firstfree, stripe; uint64_t bytes; int nitems, choice, junk; @@ -1078,7 +1089,7 @@ gpart_create(struct gprovider *pp, const char *default_type, /* Default options */ strncpy(options_fstype, items[0].text, sizeof(options_fstype)); - newfs_command(options_fstype, newfs, 1); + newfs = newfs_command(options_fstype, 1); addpartform: if (interactive) { dialog_vars.extra_label = "Options"; @@ -1092,9 +1103,10 @@ addpartform: case 1: /* Cancel */ return; case 3: /* Options */ + free(newfs); strncpy(options_fstype, items[0].text, sizeof(options_fstype)); - newfs_command(options_fstype, newfs, 0); + newfs = newfs_command(options_fstype, 0); goto addpartform; } } @@ -1104,8 +1116,9 @@ addpartform: * their choices in favor of the new filesystem's defaults. */ if (strcmp(options_fstype, items[0].text) != 0) { + free(newfs); strncpy(options_fstype, items[0].text, sizeof(options_fstype)); - newfs_command(options_fstype, newfs, 1); + newfs = newfs_command(options_fstype, 1); } size = maxsize; @@ -1250,6 +1263,7 @@ addpartform: else set_default_part_metadata(newpartname, scheme, items[0].text, items[2].text, newfs); + free(newfs); for (i = 0; i < nitems(items); i++) if (items[i].text_free)