From aba49d7a2072330cea919693839c2394ef8937b4 Mon Sep 17 00:00:00 2001 From: Dan Cross Date: Fri, 12 Oct 2018 20:30:23 +0000 Subject: [PATCH 1/3] Added `file2stralloc` to read a file directly into a `stralloc`. Signed-off-by: Dan Cross --- src/bbs.h | 1 + src/util.c | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/bbs.h b/src/bbs.h index 5c32627..e90fc9d 100644 --- a/src/bbs.h +++ b/src/bbs.h @@ -406,6 +406,7 @@ extern char **split_on_space(char *str, size_t *lenp); extern void die(const char *msg); extern void *malloz(size_t size); extern char *file2str(const char *path); +extern stralloc file2stralloc(const char *path); extern char *str5dup(const char *a, const char *b, const char *c, const char *d, const char *e); extern char *str4dup(const char *a, const char *b, const char *c, const char *d); extern char *str3dup(const char *a, const char *b, const char *c); diff --git a/src/util.c b/src/util.c index 03b6f46..53c195f 100644 --- a/src/util.c +++ b/src/util.c @@ -1,4 +1,5 @@ #include +#include #include #include @@ -45,6 +46,32 @@ char *file2str(const char *path) { return contents; } +stralloc file2stralloc(const char *path) { + struct stat s; + int fd; + + memset(&s, 0, sizeof(s)); + if (stat(path, &s) < 0) + return EMPTY_STRALLOC; + if (!S_ISREG(s.st_mode)) + return EMPTY_STRALLOC; + fd = open(path, O_RDONLY); + if (fd < 0) + return EMPTY_STRALLOC; + size_t len = s.st_size; + char *p = mmap(NULL, len, PROT_READ, MAP_SHARED, fd, 0); + if (p == NULL) { + close(fd); + return EMPTY_STRALLOC; + } + stralloc sa = EMPTY_STRALLOC; + stralloc_copyb(&sa, p, len); + munmap(fd, len); + close(fd); + + return sa; +} + char *str5dup(const char *a, const char *b, const char *c, const char *d, const char *e) { char *p; size_t alen, blen, clen, dlen, elen; From 7bf6e05170a355da9f0a631517284f5159a79c13 Mon Sep 17 00:00:00 2001 From: Dan Cross Date: Fri, 12 Oct 2018 20:30:24 +0000 Subject: [PATCH 2/3] Fix a buffer overflow in bluewave.c. strcat()'ing a string onto the result of file2str() will result in a buffer overflow, since file2str() only allocates enough memory to hold the contents of the file (plus a NUL terminator). This happend in `bluewave.c`. Instead, use `file2stralloc` to read the contents of that file into a stralloc, which we can stralloc_cats onto without fear of overflow. Signed-off-by: Dan Cross --- src/bluewave.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/bluewave.c b/src/bluewave.c index fc0c696..83fb5c5 100644 --- a/src/bluewave.c +++ b/src/bluewave.c @@ -778,6 +778,7 @@ void bwave_upload_reply() { int area; tWORD msg_attr; struct fido_addr addr; + stralloc sa = EMPTY_STRALLOC; char *body; char *tagline; struct stat s; @@ -1053,20 +1054,20 @@ void bwave_upload_reply() { snprintf(originlinebuffer, 256, "\r"); } - body = file2str(msgbuffer); - if (body == NULL) { + sa = file2stralloc(msgbuffer); + if (sa.s == NULL) { continue; } + stralloc_cats(&sa, originlinebuffer); + stralloc_0(&sa); + body = sa.s; + char *p, *s; - strcat(body, originlinebuffer); - - bpos = 0; - for (i = 0; i < strlen(body); i++) { - if (body[i] != '\n') { - body[bpos++] = body[i]; - } + for (p = s = body; *p != '\0'; ++p) { + if (*p != '\n') + *s++ = *p; } - body[bpos] = '\0'; + *s = '\0'; if (bwave_add_message(confr, area, convertl(upl_rec.unix_date), upl_rec.to, upl_rec.subj, &addr, body) != 0) { // failed to add message From f2288e9cec7aaf9bd59393f649c5fb5aa090c0d1 Mon Sep 17 00:00:00 2001 From: Dan Cross Date: Fri, 12 Oct 2018 20:30:25 +0000 Subject: [PATCH 3/3] More adoption of stralloc for string handling. Signed-off-by: Dan Cross --- src/files.c | 46 ++++++++++++++++++++++------------------------ src/lua_glue.c | 35 ++++++++++++++--------------------- 2 files changed, 36 insertions(+), 45 deletions(-) diff --git a/src/files.c b/src/files.c index 392708a..5fa538c 100644 --- a/src/files.c +++ b/src/files.c @@ -701,10 +701,10 @@ int do_upload(struct user_record *user, char *final_path) { } } } + void upload(struct user_record *user) { - char buffer[331]; - char buffer2[66]; - char buffer3[256]; + stralloc buffer = EMPTY_STRALLOC; + char pathname[PATH_MAX]; int i; char *create_sql = "CREATE TABLE IF NOT EXISTS files (" @@ -735,25 +735,27 @@ void upload(struct user_record *user) { description = get_file_id_diz(upload_filename); if (description == NULL) { + char descbuf[66]; + s_printf(get_string(199)); s_printf(get_string(200)); - buffer[0] = '\0'; for (i = 0; i < 5; i++) { s_printf("\r\n%d: ", i); - s_readstring(buffer2, 65); - if (strlen(buffer2) == 0) { + s_readstring(descbuf, sizeof(descbuf) - 1); + if (*descbuf == '\0') { break; } - strcat(buffer, buffer2); - strcat(buffer, "\n"); + stralloc_cats(&buffer, descbuf); + stralloc_append1(&buffer, '\n'); } - + stralloc_0(&buffer); } else { s_printf(get_string(201)); } - sprintf(buffer3, "%s/%s.sq3", conf.bbs_path, conf.file_directories[user->cur_file_dir]->file_subs[user->cur_file_sub]->database); + snprintf(pathname, sizeof pathname, "%s/%s.sq3", + conf.bbs_path, conf.file_directories[user->cur_file_dir]->file_subs[user->cur_file_sub]->database); - rc = sqlite3_open(buffer3, &db); + rc = sqlite3_open(pathname, &db); if (rc != SQLITE_OK) { dolog("Cannot open database: %s", sqlite3_errmsg(db)); @@ -766,9 +768,8 @@ void upload(struct user_record *user) { dolog("SQL error: %s", err_msg); sqlite3_free(err_msg); sqlite3_close(db); - if (description != NULL) { - free(description); - } + free(description); + free(buffer.s); return; } rc = sqlite3_prepare_v2(db, sql, -1, &res, 0); @@ -778,7 +779,7 @@ void upload(struct user_record *user) { sqlite3_bind_text(res, 1, upload_filename, -1, 0); if (description == NULL) { - sqlite3_bind_text(res, 2, buffer, -1, 0); + sqlite3_bind_text(res, 2, buffer.s, -1, 0); } else { sqlite3_bind_text(res, 2, description, -1, 0); } @@ -789,9 +790,8 @@ void upload(struct user_record *user) { dolog("Failed to execute statement: %s", sqlite3_errmsg(db)); sqlite3_finalize(res); sqlite3_close(db); - if (description != NULL) { - free(description); - } + free(description); + free(buffer.s); return; } @@ -801,16 +801,14 @@ void upload(struct user_record *user) { dolog("execution failed: %s", sqlite3_errmsg(db)); sqlite3_finalize(res); sqlite3_close(db); - if (description != NULL) { - free(description); - } + free(description); + free(buffer.s); return; } sqlite3_finalize(res); sqlite3_close(db); - if (description != NULL) { - free(description); - } + free(description); + free(buffer.s); s_printf(get_string(202)); s_printf(get_string(6)); diff --git a/src/lua_glue.c b/src/lua_glue.c index 2e7655f..16183c2 100644 --- a/src/lua_glue.c +++ b/src/lua_glue.c @@ -395,7 +395,6 @@ int l_postMessage(lua_State *L) { int z; int j; int i; - char *msg; char *tagline; struct utsname name; @@ -514,21 +513,16 @@ int l_postMessage(lua_State *L) { snprintf(buffer, 256, "\r--- MagickaBBS v%d.%d%s (%s/%s)\r * Origin: %s \r", VERSION_MAJOR, VERSION_MINOR, VERSION_STR, name.sysname, name.machine, tagline); } - msg = (char *)malloz(strlen(body) + 2 + strlen(buffer)); + stralloc sa = EMPTY_STRALLOC; + stralloc_ready(&sa, strlen(body) + 2 + strlen(buffer)); + for (char *p = body; *p != '\0'; ++p) + if (*p != '\n') + stralloc_append1(&sa, *p); + stralloc_cats(&sa, buffer); + stralloc_0(&sa); + char *msg = sa.s; - j = 0; - - for (i = 0; i < strlen(body); i++) { - if (body[i] == '\n') { - continue; - } - msg[j++] = body[i]; - } - msg[j] = '\0'; - - strcat(msg, buffer); - - if (JAM_AddMessage(jb, &jmh, jsp, (char *)msg, strlen(msg))) { + if (JAM_AddMessage(jb, &jmh, jsp, msg, strlen(msg))) { dolog("Failed to add message"); JAM_UnlockMB(jb); @@ -536,13 +530,12 @@ int l_postMessage(lua_State *L) { JAM_CloseMB(jb); free(msg); return 0; - } else { - JAM_UnlockMB(jb); - - JAM_DelSubPacket(jsp); - JAM_CloseMB(jb); - free(jb); } + JAM_UnlockMB(jb); + + JAM_DelSubPacket(jsp); + JAM_CloseMB(jb); + free(jb); free(msg); if (conf.mail_conferences[confr]->mail_areas[area]->type == TYPE_ECHOMAIL_AREA || conf.mail_conferences[confr]->mail_areas[area]->type == TYPE_NEWSGROUP_AREA) {