Date: Thu, 2 May 2024 09:06:25 GMT From: Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= <des@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 986cceda16d8 - stable/14 - tftpd: Use `size_t` where appropriate. Message-ID: <202405020906.44296P2Z039779@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=986cceda16d8de091524314b50325d52fe403d75 commit 986cceda16d8de091524314b50325d52fe403d75 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-04-25 18:35:15 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-05-02 09:02:21 +0000 tftpd: Use `size_t` where appropriate. * Limit the use of `ssize_t` to only where it's needed. * Correct one case of `int` being used for a length. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: kevans Differential Revision: https://reviews.freebsd.org/D44954 (cherry picked from commit 1ed44fcc44b2c04db330663589541608135402f4) tftpd: Clean up the tests. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: kevans Differential Revision: https://reviews.freebsd.org/D44955 (cherry picked from commit 7ab7ecfcfe777f7816e3e01df5f277060b2b609a) tftpd: Check the server status after each test. * In the setup phase, wait for the server to start (or fail to start) before proceeding with the test. This makes it possible to write test cases that don't expect a response from the server without ending up in a race over the server PID file. * After running each test, wait up to 30 seconds for the server to exit and check that the exit status matches what the test case says to expect (usually 0). * We still kill and collect the server in the cleanup phase, in case the test ended early. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: kevans Differential Revision: https://reviews.freebsd.org/D44956 (cherry picked from commit 83a6e984ac01657819418746f722163367ec30db) tftpd: Immediately reject any request shorter than 4 bytes. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: kevans Differential Revision: https://reviews.freebsd.org/D44957 (cherry picked from commit 9f231af307b80eb222d9761bbd81fa4e130bb3d7) tftpd: Untangle a conditional. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D45026 (cherry picked from commit 21b5829d28b1e02e9f30bfa439238c382dd19114) --- libexec/tftpd/tests/functional.c | 352 +++++++++++++++++++++++---------------- libexec/tftpd/tftp-utils.c | 4 +- libexec/tftpd/tftp-utils.h | 2 +- libexec/tftpd/tftpd.c | 30 ++-- 4 files changed, 233 insertions(+), 155 deletions(-) diff --git a/libexec/tftpd/tests/functional.c b/libexec/tftpd/tests/functional.c index 1d5c1e17c420..f31d2a893da1 100644 --- a/libexec/tftpd/tests/functional.c +++ b/libexec/tftpd/tests/functional.c @@ -52,8 +52,8 @@ static bool s_flag = false; /* Pass -s to tftpd */ static bool w_flag = false; /* Pass -w to tftpd */ /* Helper functions*/ -static void require_bufeq(const char *expected, ssize_t expected_len, - const char *actual, ssize_t len); +static void require_bufeq(const char *expected, size_t expected_len, + const char *actual, size_t len); /* * Receive a response from tftpd @@ -61,24 +61,24 @@ static void require_bufeq(const char *expected, ssize_t expected_len, * @param contents The reply's expected contents, as a char array * @param contents_len Length of contents */ -#define RECV(hdr, contents, contents_len) do { \ - char buffer[1024]; \ - struct sockaddr_storage from; \ - socklen_t fromlen = sizeof(from); \ - ssize_t r = recvfrom(s, buffer, sizeof(buffer), 0, \ - (struct sockaddr*)&from, &fromlen); \ - ATF_REQUIRE(r > 0); \ - require_bufeq((hdr), sizeof(hdr), buffer, \ - MIN(r, (ssize_t)sizeof(hdr))); \ - require_bufeq((const char*) (contents), (contents_len), \ - &buffer[sizeof(hdr)], r - sizeof(hdr)); \ - if (protocol == PF_INET) { \ - ((struct sockaddr_in*)&addr)->sin_port = \ - ((struct sockaddr_in*)&from)->sin_port; \ - } else { \ - ((struct sockaddr_in6*)&addr)->sin6_port = \ - ((struct sockaddr_in6*)&from)->sin6_port; \ - } \ +#define RECV(hdr, contents, contents_len) do { \ + char buffer[1024]; \ + struct sockaddr_storage from; \ + socklen_t fromlen = sizeof(from); \ + ssize_t r = recvfrom(s, buffer, sizeof(buffer), 0, \ + (struct sockaddr *)&from, &fromlen); \ + ATF_REQUIRE(r > 0); \ + require_bufeq((hdr), sizeof(hdr), buffer, \ + MIN((size_t)r, sizeof(hdr))); \ + require_bufeq((const char *) (contents), (contents_len), \ + &buffer[sizeof(hdr)], r - sizeof(hdr)); \ + if (protocol == PF_INET) { \ + ((struct sockaddr_in *)&addr)->sin_port = \ + ((struct sockaddr_in *)&from)->sin_port; \ + } else { \ + ((struct sockaddr_in6 *)&addr)->sin6_port = \ + ((struct sockaddr_in6 *)&from)->sin6_port; \ + } \ } while(0) static void @@ -102,15 +102,15 @@ recv_oack(const char *options, size_t options_len) * @param contents_len Length of contents expected to receive */ static void -recv_data(uint16_t blocknum, const char* contents, size_t contents_len) +recv_data(uint16_t blocknum, const char *contents, size_t contents_len) { char hdr[] = {0, 3, blocknum >> 8, blocknum & 0xFF}; RECV(hdr, contents, contents_len); } -#define RECV_ERROR(code, msg) do { \ - char hdr[] = {0, 5, code >> 8, code & 0xFF}; \ - RECV(hdr, msg, sizeof(msg)); \ +#define RECV_ERROR(code, msg) do { \ + char hdr[] = {0, 5, code >> 8, code & 0xFF}; \ + RECV(hdr, msg, sizeof(msg)); \ } while (0) /* @@ -118,16 +118,17 @@ recv_data(uint16_t blocknum, const char* contents, size_t contents_len) * @param cmd Command to send, as a char array */ static void -send_bytes(const void* cmd, ssize_t len) +send_bytes(const void *cmd, size_t len) { ssize_t r; - r = sendto(s, cmd, len, 0, (struct sockaddr*)(&addr), addr.ss_len); - ATF_REQUIRE_EQ(r, len); + r = sendto(s, cmd, len, 0, (struct sockaddr *)(&addr), addr.ss_len); + ATF_REQUIRE(r >= 0); + ATF_REQUIRE_EQ(len, (size_t)r); } static void -send_data(uint16_t blocknum, const char* contents, size_t contents_len) +send_data(uint16_t blocknum, const char *contents, size_t contents_len) { char buffer[1024]; @@ -144,10 +145,10 @@ send_data(uint16_t blocknum, const char* contents, size_t contents_len) * @param cmd Command to send, as a const string * (terminating NUL will be ignored) */ -#define SEND_STR(cmd) ATF_REQUIRE_EQ( \ - sendto(s, (cmd), sizeof(cmd) - 1, 0, (struct sockaddr*)(&addr), \ - addr.ss_len), \ - sizeof(cmd) - 1) +#define SEND_STR(cmd) \ + ATF_REQUIRE_EQ(sizeof(cmd) - 1, \ + sendto(s, (cmd), sizeof(cmd) - 1, 0, \ + (struct sockaddr *)(&addr), addr.ss_len)) /* * Acknowledge block blocknum @@ -156,13 +157,12 @@ static void send_ack(uint16_t blocknum) { char packet[] = { - 0, 4, /* ACK opcode in BE */ - blocknum >> 8, - blocknum & 0xFF + 0, 4, /* ACK opcode in BE */ + blocknum >> 8, + blocknum & 0xFF }; send_bytes(packet, sizeof(packet)); - } /* @@ -175,73 +175,109 @@ send_ack(uint16_t blocknum) * @param filename filename as a string, absolute or relative * @param mode either "octet" or "netascii" */ -#define SEND_RRQ(filename, mode) SEND_STR("\0\001" filename "\0" mode "\0") +#define SEND_RRQ(filename, mode) \ + SEND_STR("\0\001" filename "\0" mode "\0") /* * send a read request with options */ -#define SEND_RRQ_OPT(filename, mode, options) SEND_STR("\0\001" filename "\0" mode "\000" options) +#define SEND_RRQ_OPT(filename, mode, options) \ + SEND_STR("\0\001" filename "\0" mode "\000" options) /* * send a write request to tftpd. * @param filename filename as a string, absolute or relative * @param mode either "octet" or "netascii" */ -#define SEND_WRQ(filename, mode) SEND_STR("\0\002" filename "\0" mode "\0") +#define SEND_WRQ(filename, mode) \ + SEND_STR("\0\002" filename "\0" mode "\0") /* * send a write request with options */ -#define SEND_WRQ_OPT(filename, mode, options) SEND_STR("\0\002" filename "\0" mode "\000" options) +#define SEND_WRQ_OPT(filename, mode, options) \ + SEND_STR("\0\002" filename "\0" mode "\000" options) /* Define a test case, for both IPv4 and IPv6 */ -#define TFTPD_TC_DEFINE(name, head, ...) \ -static void \ -name ## _body(void); \ -ATF_TC_WITH_CLEANUP(name ## _v4); \ -ATF_TC_HEAD(name ## _v4, tc) \ -{ \ - head \ -} \ -ATF_TC_BODY(name ## _v4, tc) \ -{ \ - __VA_ARGS__; \ - protocol = AF_INET; \ - s = setup(&addr, __COUNTER__); \ - name ## _body(); \ - close(s); \ -} \ -ATF_TC_CLEANUP(name ## _v4, tc) \ -{ \ - cleanup(); \ -} \ -ATF_TC_WITH_CLEANUP(name ## _v6); \ -ATF_TC_HEAD(name ## _v6, tc) \ -{ \ - head \ -} \ -ATF_TC_BODY(name ## _v6, tc) \ -{ \ - __VA_ARGS__; \ - protocol = AF_INET6; \ - s = setup(&addr, __COUNTER__); \ - name ## _body(); \ - close(s); \ -} \ -ATF_TC_CLEANUP(name ## _v6, tc) \ -{ \ - cleanup(); \ -} \ -static void \ +#define TFTPD_TC_DEFINE(name, head, ...) \ +static void \ +name ## _body(void); \ +ATF_TC_WITH_CLEANUP(name ## _v4); \ +ATF_TC_HEAD(name ## _v4, tc) \ +{ \ + head \ +} \ +ATF_TC_BODY(name ## _v4, tc) \ +{ \ + int exitcode = 0; \ + __VA_ARGS__; \ + protocol = AF_INET; \ + s = setup(&addr, __COUNTER__); \ + name ## _body(); \ + close(s); \ + if (exitcode >= 0) \ + check_server(exitcode); \ +} \ +ATF_TC_CLEANUP(name ## _v4, tc) \ +{ \ + cleanup(); \ +} \ +ATF_TC_WITH_CLEANUP(name ## _v6); \ +ATF_TC_HEAD(name ## _v6, tc) \ +{ \ + head \ +} \ +ATF_TC_BODY(name ## _v6, tc) \ +{ \ + int exitcode = 0; \ + __VA_ARGS__; \ + protocol = AF_INET6; \ + s = setup(&addr, __COUNTER__); \ + name ## _body(); \ + close(s); \ + if (exitcode >= 0) \ + check_server(exitcode); \ +} \ +ATF_TC_CLEANUP(name ## _v6, tc) \ +{ \ + cleanup(); \ +} \ +static void \ name ## _body(void) /* Add the IPv4 and IPv6 versions of a test case */ -#define TFTPD_TC_ADD(tp, name ) \ -do { \ - ATF_TP_ADD_TC(tp, name ## _v4); \ - ATF_TP_ADD_TC(tp, name ## _v6); \ +#define TFTPD_TC_ADD(tp, name) do { \ + ATF_TP_ADD_TC(tp, name ## _v4); \ + ATF_TP_ADD_TC(tp, name ## _v6); \ } while (0) +static void +sigalrm(int signo __unused) +{ +} + +/* Check that server exits with specific exit code */ +static void +check_server(int exitcode) +{ + struct sigaction sa = { .sa_handler = sigalrm }; + struct itimerval it = { .it_value = { .tv_sec = 30 } }; + FILE *f; + pid_t pid; + int wstatus; + + f = fopen(pidfile, "r"); + ATF_REQUIRE(f != NULL); + ATF_REQUIRE_INTEQ(1, fscanf(f, "%d", &pid)); + ATF_CHECK_INTEQ(0, fclose(f)); + ATF_REQUIRE_INTEQ(0, sigaction(SIGALRM, &sa, NULL)); + ATF_REQUIRE_EQ(0, setitimer(ITIMER_REAL, &it, NULL)); + ATF_REQUIRE_EQ(pid, waitpid(pid, &wstatus, 0)); + ATF_CHECK(WIFEXITED(wstatus)); + ATF_CHECK_INTEQ(exitcode, WEXITSTATUS(wstatus)); + unlink(pidfile); +} + /* Standard cleanup used by all testcases */ static void cleanup(void) @@ -252,26 +288,26 @@ cleanup(void) f = fopen(pidfile, "r"); if (f == NULL) return; + unlink(pidfile); if (fscanf(f, "%d", &pid) == 1) { kill(pid, SIGTERM); waitpid(pid, NULL, 0); } fclose(f); - unlink(pidfile); } /* Assert that two binary buffers are identical */ static void -require_bufeq(const char *expected, ssize_t expected_len, const char *actual, - ssize_t len) +require_bufeq(const char *expected, size_t expected_len, + const char *actual, size_t len) { - ssize_t i; + size_t i; ATF_REQUIRE_EQ_MSG(expected_len, len, - "Expected %zd bytes but got %zd", expected_len, len); + "Expected %zu bytes but got %zu", expected_len, len); for (i = 0; i < len; i++) { - ATF_REQUIRE_EQ_MSG(actual[i], expected[i], - "Expected %#hhx at position %zd; got %hhx instead", + ATF_REQUIRE_EQ_MSG(expected[i], actual[i], + "Expected %#hhx at position %zu; got %hhx instead", expected[i], i, actual[i]); } } @@ -297,6 +333,9 @@ setup(struct sockaddr_storage *to, uint16_t idx) struct pidfh *pfh; uint16_t port = BASEPORT + idx; socklen_t len; + int pd[2]; + + ATF_REQUIRE_EQ(0, pipe2(pd, O_CLOEXEC)); if (protocol == PF_INET) { len = sizeof(addr4); @@ -304,17 +343,17 @@ setup(struct sockaddr_storage *to, uint16_t idx) addr4.sin_len = len; addr4.sin_family = PF_INET; addr4.sin_port = htons(port); - server_addr = (struct sockaddr*)&addr4; + server_addr = (struct sockaddr *)&addr4; } else { len = sizeof(addr6); bzero(&addr6, len); addr6.sin6_len = len; addr6.sin6_family = PF_INET6; addr6.sin6_port = htons(port); - server_addr = (struct sockaddr*)&addr6; + server_addr = (struct sockaddr *)&addr6; } - ATF_REQUIRE_EQ(getcwd(pwd, sizeof(pwd)), pwd); + ATF_REQUIRE_EQ(pwd, getcwd(pwd, sizeof(pwd))); /* Must bind(2) pre-fork so it happens before the client's send(2) */ server_s = socket(protocol, SOCK_DGRAM, 0); @@ -324,7 +363,7 @@ setup(struct sockaddr_storage *to, uint16_t idx) } ATF_REQUIRE_MSG(server_s >= 0, "socket failed with error %s", strerror(errno)); - ATF_REQUIRE_EQ_MSG(bind(server_s, server_addr, len), 0, + ATF_REQUIRE_EQ_MSG(0, bind(server_s, server_addr, len), "bind failed with error %s", strerror(errno)); pid = fork(); @@ -337,8 +376,8 @@ setup(struct sockaddr_storage *to, uint16_t idx) pfh = pidfile_open(pidfile, 0644, NULL); ATF_REQUIRE_MSG(pfh != NULL, "pidfile_open: %s", strerror(errno)); - ATF_REQUIRE_EQ(pidfile_write(pfh), 0); - ATF_REQUIRE_EQ(pidfile_close(pfh), 0); + ATF_REQUIRE_EQ(0, pidfile_write(pfh)); + ATF_REQUIRE_EQ(0, pidfile_close(pfh)); bzero(argv, sizeof(argv)); argv[0] = execname; @@ -348,31 +387,35 @@ setup(struct sockaddr_storage *to, uint16_t idx) if (s_flag) argv[argv_idx++] = s_flag_str; argv[argv_idx++] = pwd; - ATF_REQUIRE_EQ(dup2(server_s, STDOUT_FILENO), STDOUT_FILENO); - ATF_REQUIRE_EQ(dup2(server_s, STDIN_FILENO), STDIN_FILENO); - ATF_REQUIRE_EQ(dup2(server_s, STDERR_FILENO), STDERR_FILENO); + ATF_REQUIRE_EQ(STDOUT_FILENO, dup2(server_s, STDOUT_FILENO)); + ATF_REQUIRE_EQ(STDIN_FILENO, dup2(server_s, STDIN_FILENO)); + ATF_REQUIRE_EQ(STDERR_FILENO, dup2(server_s, STDERR_FILENO)); execv(execname, argv); atf_tc_fail("exec failed"); break; default: /* In parent */ + ATF_REQUIRE_INTEQ(0, close(pd[1])); + /* block until other end is closed on exec() or exit() */ + ATF_REQUIRE_INTEQ(0, read(pd[0], &pd[1], sizeof(pd[1]))); + ATF_REQUIRE_INTEQ(0, close(pd[0])); bzero(to, sizeof(*to)); if (protocol == PF_INET) { - struct sockaddr_in *to4 = (struct sockaddr_in*)to; + struct sockaddr_in *to4 = (struct sockaddr_in *)to; to4->sin_len = sizeof(*to4); to4->sin_family = PF_INET; to4->sin_port = htons(port); to4->sin_addr.s_addr = htonl(INADDR_LOOPBACK); } else { struct in6_addr loopback = IN6ADDR_LOOPBACK_INIT; - struct sockaddr_in6 *to6 = (struct sockaddr_in6*)to; + struct sockaddr_in6 *to6 = (struct sockaddr_in6 *)to; to6->sin6_len = sizeof(*to6); to6->sin6_family = PF_INET6; to6->sin6_port = htons(port); to6->sin6_addr = loopback; } - close(server_s); + ATF_REQUIRE_INTEQ(0, close(server_s)); ATF_REQUIRE((client_s = socket(protocol, SOCK_DGRAM, 0)) > 0); break; } @@ -392,8 +435,8 @@ write_all(int fd, const void *buf, size_t nbytes) while (nbytes > 0) { r = write(fd, buf, nbytes); ATF_REQUIRE(r > 0); - nbytes -= r; - buf = (const char*)buf + r; + nbytes -= (size_t)r; + buf = (const char *)buf + (size_t)r; } } @@ -433,13 +476,13 @@ TFTPD_TC_DEFINE(abspath,) */ TFTPD_TC_DEFINE(dotdot,) { - ATF_REQUIRE_EQ(mkdir("subdir", 0777), 0); + ATF_REQUIRE_EQ(0, mkdir("subdir", 0777)); SEND_RRQ("../disallowed.txt", "octet"); RECV_ERROR(2, "Access violation"); - s = setup(&addr, __COUNTER__); \ + s = setup(&addr, __COUNTER__); SEND_RRQ("subdir/../../disallowed.txt", "octet"); RECV_ERROR(2, "Access violation"); - s = setup(&addr, __COUNTER__); \ + s = setup(&addr, __COUNTER__); SEND_RRQ("/etc/passwd", "octet"); RECV_ERROR(2, "Access violation"); } @@ -447,8 +490,9 @@ TFTPD_TC_DEFINE(dotdot,) /* * With "-s", tftpd should chroot to the specified directory */ -TFTPD_TC_DEFINE(s_flag, atf_tc_set_md_var(tc, "require.user", "root");, - s_flag = true) +TFTPD_TC_DEFINE(s_flag, + atf_tc_set_md_var(tc, "require.user", "root");, + s_flag = true) { int fd; char contents[] = "small"; @@ -505,7 +549,7 @@ TFTPD_TC_DEFINE(rrq_dropped_data,) close(fd); SEND_RRQ("medium.txt", "octet"); - recv_data(1, (const char*)&contents[0], 512); + recv_data(1, (const char *)&contents[0], 512); send_ack(1); (void) recvfrom(s, buffer, sizeof(buffer), 0, NULL, NULL); /* @@ -513,7 +557,7 @@ TFTPD_TC_DEFINE(rrq_dropped_data,) * Eventually, client should resend the last ACK */ send_ack(1); - recv_data(2, (const char*)&contents[128], 256); + recv_data(2, (const char *)&contents[128], 256); send_ack(2); } @@ -535,11 +579,11 @@ TFTPD_TC_DEFINE(rrq_duped_ack,) close(fd); SEND_RRQ("medium.txt", "octet"); - recv_data(1, (const char*)&contents[0], 512); + recv_data(1, (const char *)&contents[0], 512); send_ack(1); send_ack(1); /* Dupe an ACK packet */ - recv_data(2, (const char*)&contents[128], 256); - recv_data(2, (const char*)&contents[128], 256); + recv_data(2, (const char *)&contents[128], 256); + recv_data(2, (const char *)&contents[128], 256); send_ack(2); } @@ -593,9 +637,9 @@ TFTPD_TC_DEFINE(rrq_medium,) close(fd); SEND_RRQ("medium.txt", "octet"); - recv_data(1, (const char*)&contents[0], 512); + recv_data(1, (const char *)&contents[0], 512); send_ack(1); - recv_data(2, (const char*)&contents[128], 256); + recv_data(2, (const char *)&contents[128], 256); send_ack(2); } @@ -620,8 +664,8 @@ TFTPD_TC_DEFINE(rrq_medium_window,) SEND_RRQ_OPT("medium.txt", "octet", OPTION_STR("windowsize", "2")); recv_oack(options, sizeof(options) - 1); send_ack(0); - recv_data(1, (const char*)&contents[0], 512); - recv_data(2, (const char*)&contents[128], 256); + recv_data(1, (const char *)&contents[0], 512); + recv_data(2, (const char *)&contents[128], 256); send_ack(2); } @@ -764,13 +808,13 @@ TFTPD_TC_DEFINE(unknown_modes,) { SEND_RRQ("foo.txt", "ascii"); /* Misspelling of "ascii" */ RECV_ERROR(4, "Illegal TFTP operation"); - s = setup(&addr, __COUNTER__); \ + s = setup(&addr, __COUNTER__); SEND_RRQ("foo.txt", "binary"); /* Obsolete. Use "octet" instead */ RECV_ERROR(4, "Illegal TFTP operation"); - s = setup(&addr, __COUNTER__); \ + s = setup(&addr, __COUNTER__); SEND_RRQ("foo.txt", "en_US.UTF-8"); RECV_ERROR(4, "Illegal TFTP operation"); - s = setup(&addr, __COUNTER__); \ + s = setup(&addr, __COUNTER__); SEND_RRQ("foo.txt", "mail"); /* Obsolete in RFC-1350 */ RECV_ERROR(4, "Illegal TFTP operation"); } @@ -805,8 +849,9 @@ TFTPD_TC_DEFINE(w_flag,, w_flag = 1;) fd = open("small.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); + ATF_REQUIRE(r > 0); close(fd); - require_bufeq(contents, contents_len, buffer, r); + require_bufeq(contents, contents_len, buffer, (size_t)r); } /* @@ -829,21 +874,22 @@ TFTPD_TC_DEFINE(wrq_dropped_ack,) SEND_WRQ("medium.txt", "octet"); recv_ack(0); - send_data(1, (const char*)&contents[0], 512); + send_data(1, (const char *)&contents[0], 512); /* * Servers "sends" an ACK packet, but network drops it. * Eventually, server should resend the last ACK */ (void) recvfrom(s, buffer, sizeof(buffer), 0, NULL, NULL); recv_ack(1); - send_data(2, (const char*)&contents[128], 256); + send_data(2, (const char *)&contents[128], 256); recv_ack(2); fd = open("medium.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); + ATF_REQUIRE(r > 0); close(fd); - require_bufeq((const char*)contents, 768, buffer, r); + require_bufeq((const char *)contents, 768, buffer, (size_t)r); } /* @@ -875,8 +921,9 @@ TFTPD_TC_DEFINE(wrq_dropped_data,) fd = open("small.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); + ATF_REQUIRE(r > 0); close(fd); - require_bufeq(contents, contents_len, buffer, r); + require_bufeq(contents, contents_len, buffer, (size_t)r); } /* @@ -899,18 +946,19 @@ TFTPD_TC_DEFINE(wrq_duped_data,) SEND_WRQ("medium.txt", "octet"); recv_ack(0); - send_data(1, (const char*)&contents[0], 512); - send_data(1, (const char*)&contents[0], 512); + send_data(1, (const char *)&contents[0], 512); + send_data(1, (const char *)&contents[0], 512); recv_ack(1); recv_ack(1); - send_data(2, (const char*)&contents[128], 256); + send_data(2, (const char *)&contents[128], 256); recv_ack(2); fd = open("medium.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); + ATF_REQUIRE(r > 0); close(fd); - require_bufeq((const char*)contents, 768, buffer, r); + require_bufeq((const char *)contents, 768, buffer, (size_t)r); } /* @@ -965,16 +1013,17 @@ TFTPD_TC_DEFINE(wrq_medium,) SEND_WRQ("medium.txt", "octet"); recv_ack(0); - send_data(1, (const char*)&contents[0], 512); + send_data(1, (const char *)&contents[0], 512); recv_ack(1); - send_data(2, (const char*)&contents[128], 256); + send_data(2, (const char *)&contents[128], 256); recv_ack(2); fd = open("medium.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); + ATF_REQUIRE(r > 0); close(fd); - require_bufeq((const char*)contents, 768, buffer, r); + require_bufeq((const char *)contents, 768, buffer, (size_t)r); } /* @@ -998,15 +1047,16 @@ TFTPD_TC_DEFINE(wrq_medium_window,) SEND_WRQ_OPT("medium.txt", "octet", OPTION_STR("windowsize", "2")); recv_oack(options, sizeof(options) - 1); - send_data(1, (const char*)&contents[0], 512); - send_data(2, (const char*)&contents[128], 256); + send_data(1, (const char *)&contents[0], 512); + send_data(2, (const char *)&contents[128], 256); recv_ack(2); fd = open("medium.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); + ATF_REQUIRE(r > 0); close(fd); - require_bufeq((const char*)contents, 768, buffer, r); + require_bufeq((const char *)contents, 768, buffer, (size_t)r); } /* @@ -1038,8 +1088,9 @@ TFTPD_TC_DEFINE(wrq_netascii,) fd = open("unix.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); + ATF_REQUIRE(r > 0); close(fd); - require_bufeq(expected, sizeof(expected), buffer, r); + require_bufeq(expected, sizeof(expected), buffer, (size_t)r); } /* @@ -1076,8 +1127,9 @@ TFTPD_TC_DEFINE(wrq_small,) fd = open("small.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); + ATF_REQUIRE(r > 0); close(fd); - require_bufeq(contents, contents_len, buffer, r); + require_bufeq(contents, contents_len, buffer, (size_t)r); } /* @@ -1099,8 +1151,8 @@ TFTPD_TC_DEFINE(wrq_truncate,) send_data(1, NULL, 0); recv_ack(1); - ATF_REQUIRE_EQ(stat("small.txt", &sb), 0); - ATF_REQUIRE_EQ(sb.st_size, 0); + ATF_REQUIRE_EQ(0, stat("small.txt", &sb)); + ATF_REQUIRE_EQ(0, sb.st_size); } /* @@ -1163,8 +1215,25 @@ TFTPD_TC_DEFINE(wrq_window_rfc7440,) fd = open("rfc7440.txt", O_RDONLY); ATF_REQUIRE(fd >= 0); r = read(fd, buffer, sizeof(buffer)); + ATF_REQUIRE(r > 0); close(fd); - require_bufeq(contents, sizeof(contents), buffer, r); + require_bufeq(contents, sizeof(contents), buffer, (size_t)r); +} + +/* + * Send less than four bytes + */ +TFTPD_TC_DEFINE(short_packet1, /* no head */, exitcode = 1) +{ + SEND_STR("\1"); +} +TFTPD_TC_DEFINE(short_packet2, /* no head */, exitcode = 1) +{ + SEND_STR("\1\2"); +} +TFTPD_TC_DEFINE(short_packet3, /* no head */, exitcode = 1) +{ + SEND_STR("\1\2\3"); } @@ -1204,6 +1273,9 @@ ATF_TP_ADD_TCS(tp) TFTPD_TC_ADD(tp, wrq_small); TFTPD_TC_ADD(tp, wrq_truncate); TFTPD_TC_ADD(tp, wrq_window_rfc7440); + TFTPD_TC_ADD(tp, short_packet1); + TFTPD_TC_ADD(tp, short_packet2); + TFTPD_TC_ADD(tp, short_packet3); return (atf_no_error()); } diff --git a/libexec/tftpd/tftp-utils.c b/libexec/tftpd/tftp-utils.c index 9754c3238d50..b309a94f7653 100644 --- a/libexec/tftpd/tftp-utils.c +++ b/libexec/tftpd/tftp-utils.c @@ -104,8 +104,8 @@ unmappedaddr(struct sockaddr_in6 *sin6) } /* Get a field from a \0 separated string */ -ssize_t -get_field(int peer, char *buffer, ssize_t size) +size_t +get_field(int peer, char *buffer, size_t size) { char *cp = buffer; diff --git a/libexec/tftpd/tftp-utils.h b/libexec/tftpd/tftp-utils.h index 3fecf1fc8696..763b3b493c7e 100644 --- a/libexec/tftpd/tftp-utils.h +++ b/libexec/tftpd/tftp-utils.h @@ -63,7 +63,7 @@ extern int acting_as_client; /* */ void unmappedaddr(struct sockaddr_in6 *sin6); -ssize_t get_field(int peer, char *buffer, ssize_t size); +size_t get_field(int peer, char *buffer, size_t size); /* * Packet types diff --git a/libexec/tftpd/tftpd.c b/libexec/tftpd/tftpd.c index ffe31927780e..78348a8c6aaf 100644 --- a/libexec/tftpd/tftpd.c +++ b/libexec/tftpd/tftpd.c @@ -80,8 +80,8 @@ static char sccsid[] = "@(#)tftpd.c 8.1 (Berkeley) 6/4/93"; #include <tcpd.h> #endif -static void tftp_wrq(int peer, char *, ssize_t); -static void tftp_rrq(int peer, char *, ssize_t); +static void tftp_wrq(int peer, char *, size_t); +static void tftp_rrq(int peer, char *, size_t); /* * Null-terminated directory prefix list for absolute pathname requests and @@ -93,7 +93,7 @@ static void tftp_rrq(int peer, char *, ssize_t); #define MAXDIRS 20 static struct dirlist { const char *name; - int len; + size_t len; } dirs[MAXDIRS+1]; static int suppress_naks; static int logging; @@ -240,6 +240,11 @@ main(int argc, char *argv[]) } getnameinfo((struct sockaddr *)&peer_sock, peer_sock.ss_len, peername, sizeof(peername), NULL, 0, NI_NUMERICHOST); + if ((size_t)n < 4 /* tftphdr */) { + tftp_log(LOG_ERR, "Rejecting %zd-byte request from %s", + n, peername); + exit(1); + } /* * Now that we have read the message out of the UDP @@ -404,7 +409,7 @@ main(int argc, char *argv[]) tp->th_opcode = ntohs(tp->th_opcode); if (tp->th_opcode == RRQ) { if (allow_ro) - tftp_rrq(peer, tp->th_stuff, n - 1); + tftp_rrq(peer, tp->th_stuff, (size_t)n - 1); else { tftp_log(LOG_WARNING, "%s read access denied", peername); @@ -412,7 +417,7 @@ main(int argc, char *argv[]) } } else if (tp->th_opcode == WRQ) { if (allow_wo) - tftp_wrq(peer, tp->th_stuff, n - 1); + tftp_wrq(peer, tp->th_stuff, (size_t)n - 1); else { tftp_log(LOG_WARNING, "%s write access denied", peername); @@ -455,7 +460,7 @@ reduce_path(char *fn) } static char * -parse_header(int peer, char *recvbuffer, ssize_t size, +parse_header(int peer, char *recvbuffer, size_t size, char **filename, char **mode) { char *cp; @@ -501,7 +506,7 @@ parse_header(int peer, char *recvbuffer, ssize_t size, * WRQ - receive a file from the client */ void -tftp_wrq(int peer, char *recvbuffer, ssize_t size) +tftp_wrq(int peer, char *recvbuffer, size_t size) { char *cp; int has_options = 0, ecode; @@ -546,7 +551,7 @@ tftp_wrq(int peer, char *recvbuffer, ssize_t size) * RRQ - send a file to the client */ void -tftp_rrq(int peer, char *recvbuffer, ssize_t size) +tftp_rrq(int peer, char *recvbuffer, size_t size) { char *cp; int has_options = 0, ecode; @@ -694,10 +699,11 @@ validate_access(int peer, char **filep, int mode) * it's a /. */ for (dirp = dirs; dirp->name != NULL; dirp++) { - if (dirp->len == 1 || - (!strncmp(filename, dirp->name, dirp->len) && - filename[dirp->len] == '/')) - break; + if (dirp->len == 1) + break; + if (strncmp(filename, dirp->name, dirp->len) == 0 && + filename[dirp->len] == '/') + break; } /* If directory list is empty, allow access to any file */ if (dirp->name == NULL && dirp != dirs)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202405020906.44296P2Z039779>