From 931c5625a3b89e83815327436e627254a7ca362f Mon Sep 17 00:00:00 2001 From: Dan Cross Date: Mon, 15 Oct 2018 14:37:46 +0000 Subject: [PATCH] Get rid of strncpy() calls. The poorly named `strncpy` was originally written to copy data into fixed-sized, disk-resident data structures in an early version of the research Unix kernel. Thus, it has peculiar semantics: it takes source and destination pointer arguments and a length and will *always* modify exactly `length` bytes in the destination buffer. If the length of the source (which is presumed to be a NUL-terminated C-stylestring) is `length` or more chars long, then the result will not be NUL terminated. If it is less than `length` bytes long, then the result will be padded with zeros up to `length`. This is all well and good for storing a file name into a fixed-width directory entry in 6th edition Unix, but it's not useful as a general-purpose string utility. Replaced with calls to strlcpy(), which always properly terminates the destination but doesn't have the additional zeroing behavior. Since the buffers that we're copying into were allocated with malloz(), and thus are guaranteed to be filled with zeros, we're not leaking data, but not double-zeroing either. A few other things were changed. Lengths of destination buffers are now given via `sizeof` instead of manifest constants. One call to `memcpy` took the length from the size of the source argument, thus possibly writing beyond the end of the destination buffer. Changed to a call to strlcpy() with length the sizeof destination. Signed-off-by: Dan Cross --- src/bluewave.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/bluewave.c b/src/bluewave.c index d59c863..55da216 100644 --- a/src/bluewave.c +++ b/src/bluewave.c @@ -89,7 +89,7 @@ int bwave_scan_email(int areano, int totmsgs, FILE *fti_file, FILE *mix_file, FI memset(&fti, 0, sizeof(FTI_REC)); strlcpy(fti.from, sqlite3_column_text(res, 0), sizeof fti.from); strlcpy(fti.to, gUser->loginname, sizeof fti.to); - strlcpy(fti.subject, sqlite3_column_text(res, 1), fti.subject); + strlcpy(fti.subject, sqlite3_column_text(res, 1), sizeof fti.subject); thetime = sqlite3_column_int(res, 2); localtime_r((time_t *)&thetime, &timeStruct); @@ -341,18 +341,17 @@ void bwave_create_packet() { area_count = 0; memset(&hdr, 0, sizeof(INF_HEADER)); - hdr.ver = PACKET_LEVEL; - strncpy(hdr.loginname, gUser->loginname, 42); - //strncpy(hdr.aliasname, gUser->loginname, 42); + strlcpy(hdr.loginname, gUser->loginname, sizeof hdr.loginname); + //strlcpy(hdr.aliasname, gUser->loginname, sizeof hdr.aliasname); snprintf(hdr.aliasname, sizeof hdr.aliasname, "%s %s", gUser->firstname, gUser->lastname); hdr.zone = converts(conf.main_aka->zone); hdr.node = converts(conf.main_aka->node); hdr.net = converts(conf.main_aka->net); hdr.point = converts(conf.main_aka->point); - strncpy(hdr.sysop, conf.sysop_name, 40); + strlcpy(hdr.sysop, conf.sysop_name, sizeof hdr.sysop); - strncpy(hdr.systemname, conf.bbs_name, 64); + strlcpy(hdr.systemname, conf.bbs_name, sizeof hdr.systemname); hdr.inf_header_len = converts(sizeof(INF_HEADER)); hdr.inf_areainfo_len = converts(sizeof(INF_AREA_INFO)); hdr.mix_structlen = converts(sizeof(MIX_REC)); @@ -423,11 +422,11 @@ void bwave_create_packet() { flags = 0; area = (INF_AREA_INFO *)malloz(sizeof(INF_AREA_INFO)); - snprintf(area->areanum, 6, "%d", area_count + 1); + snprintf(area->areanum, sizeof area->areanum, "%d", area_count + 1); - memcpy(area->echotag, conf.mail_conferences[i]->mail_areas[j]->qwkname, strlen(conf.mail_conferences[i]->mail_areas[j]->qwkname)); + strlcpy(area->echotag, conf.mail_conferences[i]->mail_areas[j]->qwkname, sizeof area->echotag); - strncpy(area->title, conf.mail_conferences[i]->mail_areas[j]->name, 49); + strlcpy(area->title, conf.mail_conferences[i]->mail_areas[j]->name, sizeof area->title); if (conf.mail_conferences[i]->mail_areas[j]->write_sec_level <= gUser->sec_level) { flags |= INF_POST;