AmendHub

Download:

jcs

/

subtext

/

amendments

/

630

binkp: Validate filename, more safety checks


jcs made amendment 630 4 days ago
--- binkp.c Sat Dec 6 15:10:18 2025 +++ binkp.c Fri Sep 18 15:13:24 2026 @@ -56,6 +56,7 @@ bool binkp_zip_decider(char *filename, size_t size); void binkp_fidopkt_processor(char *filename, unsigned char *data, size_t size); ssize_t binkp_buffer_file(Str255 path, char **data); +bool binkp_valid_filename(char *filename); void binkp_toss_inbox(void); void binkp_deliver_outbox(void); @@ -157,11 +158,12 @@ binkp_poll(void) elapsed == 1 ? "" : "s"); done: - if (!binkpc || binkpc->error) - binkp_last_poll_error = true; + if (binkpc != NULL) { + if (binkpc->error) + binkp_last_poll_error = true; - if (binkpc != NULL) binkp_free(); + } binkp_next_poll = Time + db->config.binkp_interval_seconds; @@ -359,7 +361,7 @@ binkp_read_frame(void) { char tmp[128]; size_t len, off, frame_data_read; - unsigned short rlen; + unsigned short rlen, slen; short error; Ptr read_dest; @@ -469,8 +471,21 @@ binkp_read_frame(void) rlen, frame_data_read, binkpc->cur_frame.data_size); #endif - if (binkpc->cur_incoming_file.frefnum == 0) - panic("binkpc: no frefnum for data, bogus state"); + if (binkpc->cur_incoming_file.frefnum == 0) { + logger_printf("[binkp] Received data frame before " + "M_FILE"); + goto failed_read; + } + + if (binkpc->cur_incoming_file.data_read > + binkpc->cur_incoming_file.size) { + logger_printf("[binkp] Received %ld bytes for %s but " + "size is %ld", binkpc->cur_incoming_file.data_read, + binkpc->cur_incoming_file.filename, + binkpc->cur_incoming_file.size); + goto failed_read; + } + len = rlen; error = FSWrite(binkpc->cur_incoming_file.frefnum, &len, binkpc->buf); @@ -525,10 +540,22 @@ binkp_read_frame(void) } } - if (sscanf(binkpc->buf + 1, "%128s %lu %lu %lu", - &binkpc->cur_incoming_file.filename, + if (sscanf(binkpc->buf + 1, "%64s %lu %lu %lu%n", + binkpc->cur_incoming_file.filename, &binkpc->cur_incoming_file.size, - &binkpc->cur_incoming_file.mtime, &off) == 4) { + &binkpc->cur_incoming_file.mtime, &off, &slen) == 4) { + if (!binkp_valid_filename( + binkpc->cur_incoming_file.filename)) { + logger_printf("[binkp] Refusing M_FILE with bad " + "filename \"%s\"", + binkpc->cur_incoming_file.filename); + binkp_send_frame(BINKP_COMMAND_M_SKIP, + binkpc->buf + 1, slen); + binkpc->cur_incoming_file.filename[0] = '\0'; + /* not an error */ + return true; + } + logger_printf("[binkp] Receiving file \"%s\" size %ld", binkpc->cur_incoming_file.filename, binkpc->cur_incoming_file.size); @@ -615,6 +642,10 @@ binkp_read_frame(void) binkpc->cur_incoming_file.filename, binkpc->cur_incoming_file.size, binkpc->cur_incoming_file.mtime); + if (len >= sizeof(tmp)) { + logger_printf("[binkp] M_GOT snprintf overflow"); + goto failed_read; + } if (!binkp_send_frame(BINKP_COMMAND_M_GOT, tmp, len)) logger_printf("[binkp] Failed sending M_GOT %s", binkpc->cur_incoming_file.filename); @@ -649,19 +680,35 @@ binkp_login(void) return false; len = snprintf(command, sizeof(command), "SYS %s", db->config.name); + if (len >= sizeof(command)) { + logger_printf("[binkp] SYS snprintf overflow"); + return false; + } if (!binkp_send_frame(BINKP_COMMAND_M_NUL, command, len)) return false; len = snprintf(command, sizeof(command), "LOC %s", db->config.location); + if (len >= sizeof(command)) { + logger_printf("[binkp] LOC snprintf overflow"); + return false; + } if (!binkp_send_frame(BINKP_COMMAND_M_NUL, command, len)) return false; len = snprintf(command, sizeof(command), "NDL 14400,TCP,BINKP"); + if (len >= sizeof(command)) { + logger_printf("[binkp] NDL snprintf overflow"); + return false; + } if (!binkp_send_frame(BINKP_COMMAND_M_NUL, command, len)) return false; len = snprintf(command, sizeof(command), "VER Subtext/%s binkp/1.0", get_version(false)); + if (len >= sizeof(command)) { + logger_printf("[binkp] VER snprintf overflow"); + return false; + } if (!binkp_send_frame(BINKP_COMMAND_M_NUL, command, len)) return false; @@ -866,7 +913,7 @@ binkp_zip_decider(char *filename, size_t size) { size_t flen = strlen(filename); - if (strcmp(filename + flen - 4, ".pkt") == 0) + if (flen >= 4 && strcmp(filename + flen - 4, ".pkt") == 0) return true; return false; @@ -976,6 +1023,31 @@ binkp_buffer_file(Str255 path, char **data) return count; } +bool +binkp_valid_filename(char *filename) +{ + size_t len, n; + unsigned char c; + + len = strlen(filename); + if (!len) + return false; + + for (n = 0; n < len; n++) { + c = filename[n]; + + if (n == 0 && c == '.') + return false; + + if (!((c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || + (c >= 'a' && c <= 'z') || c == '_' || c == '-' || c == '+' || + (c == '.'))) + return false; + } + + return true; +} + void binkp_deliver_outbox(void) { @@ -1038,6 +1110,10 @@ binkp_deliver_outbox(void) len = snprintf(command, sizeof(command), "%s %lu %lu 0", file_name_c, sb.st_size, MAC_TO_UNIX_TIME(sb.st_ctime)); + if (len >= sizeof(command)) { + logger_printf("[binkp] M_FILE snprintf overflow"); + goto done; + } if (!binkp_send_frame(BINKP_COMMAND_M_FILE, command, len)) goto done;