From dfb1f7ccf19d07155adebc0fbd8308719fe91e69 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Tue, 30 Mar 2021 21:21:09 +0100 Subject: [PATCH] TFTP tweak. Check sender of all received packets, as specified in RFC 1350 para 4. My understanding of the example in the RFC is that it in fact only applies to server-to-client packets, and packet loss or duplication cannot result in a client sending from more than one port to a server. This check is not, therefore, strictly needed on the server side. It's still useful, and adds a little security against packet spoofing. (though if you're running TFTP on a public network with bad actors, nothing can really save you.) --- CHANGELOG | 3 +++ src/tftp.c | 24 +++++++++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 6085416..1af593f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -65,6 +65,9 @@ version 2.85 Scale the size of the DNS random-port pool based on the value of the --dns-forward-max configuration. + Tweak TFTP code to check sender of all recieved packets, as + specified in RFC 1350 para 4. + version 2.84 Fix a problem, introduced in 2.83, which could see DNS replies diff --git a/src/tftp.c b/src/tftp.c index 62c3d3a..b51abb9 100644 --- a/src/tftp.c +++ b/src/tftp.c @@ -39,6 +39,7 @@ static void sanitise(char *buf); #define ERR_PERM 2 #define ERR_FULL 3 #define ERR_ILL 4 +#define ERR_TID 5 void tftp_request(struct listener *listen, time_t now) { @@ -583,11 +584,27 @@ void check_tftp_listeners(time_t now) for (transfer = daemon->tftp_trans; transfer; transfer = transfer->next) if (poll_check(transfer->sockfd, POLLIN)) { + union mysockaddr peer; + socklen_t addr_len = sizeof(union mysockaddr); + ssize_t len; + /* we overwrote the buffer... */ daemon->srv_save = NULL; - handle_tftp(now, transfer, recv(transfer->sockfd, daemon->packet, daemon->packet_buff_sz, 0)); + + if ((len = recvfrom(transfer->sockfd, daemon->packet, daemon->packet_buff_sz, 0, &peer.sa, &addr_len)) > 0) + { + if (sockaddr_isequal(&peer, &transfer->peer)) + handle_tftp(now, transfer, len); + else + { + /* Wrong source address. See rfc1350 para 4. */ + prettyprint_addr(&peer, daemon->addrbuff); + len = tftp_err(ERR_TID, daemon->packet, _("ignoring packet from %s (TID mismatch)"), daemon->addrbuff); + sendto(transfer->sockfd, daemon->packet, len, 0, &peer.sa, sa_len(&peer)); + } + } } - + for (transfer = daemon->tftp_trans, up = &daemon->tftp_trans; transfer; transfer = tmp) { tmp = transfer->next; @@ -736,7 +753,8 @@ static ssize_t tftp_err(int err, char *packet, char *message, char *file) char *errstr = strerror(errno); memset(packet, 0, daemon->packet_buff_sz); - sanitise(file); + if (file) + sanitise(file); mess->op = htons(OP_ERR); mess->err = htons(err);