Ubuntu
tftp-hpa package

tftpd-hpa sends a confusing error message if the filename is zero-length.

Bug #1946550 reported by Chris Sloan on 2021-10-09

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	tftp-hpa (Ubuntu)	Confirmed	Undecided	Unassigned

Bug Description

We were getting an error message like this in the logs:

Oct 9 01:21:39 hostname in.tftpd[192773]: sending NAK (4, Missing mode) to 10.0.0.10

This caused much confusion until we realized that the problem was our TFTP client was sending an RRQ message with a zero-length filename. Looking at the code, it doesn't check that case so it gives the wrong error about the mode instead.

I took a look at the source and I propose this patch to fix the problem:

diff --git a/tftpd/tftpd.c b/tftpd/tftpd.c
index 364e7d2..e2659e0 100644
--- a/tftpd/tftpd.c
+++ b/tftpd/tftpd.c
@@ -1085,6 +1085,11 @@ int tftp(struct tftphdr *tp, int size)

end = (char *)tp + size;

+ if (!*cp) {
+ nak(EBADOP, "Zero-length filename.");
+ exit(0);
+ }
+
     while (cp < end && *cp) {
         do {
             cp++;

$ lsb_release -rd
Description: Ubuntu 20.04.2 LTS
Release: 20.04
$ apt-cache policy tftpd-hpa
tftpd-hpa:
  Installed: 5.2+20150808-1ubuntu4
  Candidate: 5.2+20150808-1ubuntu4
  Version table:
*** 5.2+20150808-1ubuntu4 500
        500 http://us.archive.ubuntu.com/ubuntu focal/main amd64 Packages
        100 /var/lib/dpkg/status

Tags:

Revision history for this message

Chris Sloan (chris.d.sloan) wrote on 2021-10-09:

Patch adding a check for a zero-length filename and returning a better error message. Edit (364 bytes, text/plain)

Attaching the patch I came up with.

Revision history for this message

Ubuntu Foundations Team Bug Bot (crichton) wrote on 2021-10-09:

The attachment "Patch adding a check for a zero-length filename and returning a better error message." seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags:

added: patch

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2021-10-11:

Hi Chris,
I agree that the current code falls through to "Missing mode" due to `while (cp < end && *cp) {` directly skipping the loop on the *cp.

Your check makes this particular case more readable.

But carrying that only as Ubuntu Delta will potentially drive automation mad that would not expect that answer. I'd like to encourage you to submit the very same upstream to be proper part of the next official version.

Upstreams readme [1] asks to discuss things on the syslinux [2] ML.
I found no other contributor hints, so I assume that would be the best place to submit it.

If you happen to do so it would be great if you could add a link to the ML-Archive entry here. That way one can better track what happened in that discussion.

Ubuntu Impish is in final freeze right now anyway and we can't do much.
But I'd hope that your change is upstream accepted and then naturally gets into Ubuntu when we pick up the new version in the next Ubuntu release.

TBH it doesn't seem severe enough to me to SRU [3] the change afterwards.

[1]: https://salsa.debian.org/ron/tftp-hpa/-/blob/debian/README
[2]: https://lists.zytor.com/syslinux
[3]: https://wiki.ubuntu.com/StableReleaseUpdates