commit bcdb46a77ed7b4ff4d9e7ca8fae79fec74b504ae from: Omar Polo date: Wed Jan 19 19:07:21 2022 UTC fail if the received packet is too long This adds extra checks to every t* function: if the client sent too much data per message, treat is an a protocol violation and terminate the connection. Regress suite still passes, this is just an extra safety check for finding wrong clients implementations. The twrite function was the only one with a check like this. commit - ae6f257a8d22174cb2326cd60d690cceea59fa8e commit + bcdb46a77ed7b4ff4d9e7ca8fae79fec74b504ae blob - e13036d70273f0b68f3d8b52fb1aa961de7e7703 blob + bb9d9fac4523bf681bf7556b71c3718ea521e992 --- kamid/client.c +++ kamid/client.c @@ -902,6 +902,9 @@ tversion(struct np_msg_header *hdr, const uint8_t *dat goto mismatch; } + if (len != 0) + goto err; + if ((dot = strchr(version, '.')) != NULL) *dot = '\0'; @@ -959,6 +962,9 @@ tattach(struct np_msg_header *hdr, const uint8_t *data return; } + if (len != 0) + goto err; + if (fid_by_id(fid) != NULL || afid != NOFID) { np_error(hdr->tag, "invalid fid or afid"); return; @@ -997,7 +1003,8 @@ tclunk(struct np_msg_header *hdr, const uint8_t *data, uint32_t fid; /* fid[4] */ - if (!NPREAD32("fid", &fid, &data, &len)) { + if (!NPREAD32("fid", &fid, &data, &len) || + len != 0) { client_send_listener(IMSG_CLOSE, NULL, 0); client_shutdown(); return; @@ -1135,6 +1142,9 @@ twalk(struct np_msg_header *hdr, const uint8_t *data, oldfd = fd; } + if (len != 0) + goto err; + /* * If fd is -1 we've reached a file, otherwise we've just * reached another directory. We must pay attention to what @@ -1222,7 +1232,8 @@ topen(struct np_msg_header *hdr, const uint8_t *data, /* fid[4] mode[1] */ if (!NPREAD32("fid", &fid, &data, &len) || - !NPREAD8("mode", &mode, &data, &len)) { + !NPREAD8("mode", &mode, &data, &len) || + len != 0) { client_send_listener(IMSG_CLOSE, NULL, 0); client_shutdown(); return; @@ -1293,7 +1304,8 @@ tcreate(struct np_msg_header *hdr, const uint8_t *data return; } if (!NPREAD32("perm", &perm, &data, &len) || - !NPREAD8("mode", &mode, &data, &len)) + !NPREAD8("mode", &mode, &data, &len) || + len != 0) goto err; if (!strcmp(name, ".") || !strcmp(name, "..") || @@ -1432,7 +1444,8 @@ tread(struct np_msg_header *hdr, const uint8_t *data, /* fid[4] offset[8] count[4] */ if (!NPREAD32("fid", &fid, &data, &len) || !NPREAD64("offset", &off, &data, &len) || - !NPREAD32("count", &count, &data, &len)) { + !NPREAD32("count", &count, &data, &len) || + len != 0) { client_send_listener(IMSG_CLOSE, NULL, 0); client_shutdown(); return; @@ -1541,7 +1554,8 @@ tstat(struct np_msg_header *hdr, const uint8_t *data, uint32_t fid; /* fid[4] */ - if (!NPREAD32("fid", &fid, &data, &len)) { + if (!NPREAD32("fid", &fid, &data, &len) || + len != 0) { client_send_listener(IMSG_CLOSE, NULL, 0); client_shutdown(); return; @@ -1602,6 +1616,9 @@ twstat(struct np_msg_header *hdr, const uint8_t *data, return; } + if (len != 0) + goto err; + if ((f = fid_by_id(fid)) == NULL) { np_error(hdr->tag, "invalid fid"); return; @@ -1753,7 +1770,8 @@ tremove(struct np_msg_header *hdr, const uint8_t *data char dirpath[PATH_MAX + 3]; /* fid[4] */ - if (!NPREAD32("fid", &fid, &data, &len)) { + if (!NPREAD32("fid", &fid, &data, &len) || + len != 0) { client_send_listener(IMSG_CLOSE, NULL, 0); client_shutdown(); return;