tftp: Replace fgets with getline

[des]

Summary

MFC after: 1 week

[markj]

usr.bin/tftp/main.c

341

Why make these static? If the function is called more than once, the buffer pointed to by line would be invalidated. It's hard to convince myself that that won't lead to a UAF somewhere else.

343–344

line is uninitalized (well, NULL) here, and even if not, the getline() is going to discard the existing string.

429–430

Same here and further below.

[des]

usr.bin/tftp/main.c

341

Some of these functions get called multiple times in a single session, and this allows them to reuse their own private line. None of them are reentrant, so this reuse is safe.

343–344

my mistake, we shouldn't be using makeargv() at all here.

[des]

usr.bin/tftp/main.c

635

This could never work btw, after makeargv() the value would be in argv[2], not argv[1].

[des]

fix

[markj]

usr.bin/tftp/main.c

341

I don't see how you know it's safe. For instance, look at the assignment to port below. argv[2] is going to be a pointer into the line buffer, and that buffer might get freed later on if setpeer() is called again, so port becomes a dangling pointer.

That said, I don't understand how that works at all today either. It looks like port should be reset to NULL in the argc != 3 case, at the very least.

[des]

usr.bin/tftp/main.c

341

Oh, I didn't realize it was being assigned to a global variable. Yes, port here is a problem for several reasons, including the fact that it does not get reset if you first do connect host1 port1 and then connect host2. It should really be managed by setpeer0().

[des]

rf

[markj]

usr.bin/tftp/main.c

334

I think this will break the default case (lport = "tftp") unfortunately: look at how xmitfile() handles port == NULL.

[des]

usr.bin/tftp/main.c

334

moved to D57105

[des]

port issues fixed in D57105

[markj]

usr.bin/tftp/main.c

355

Doesn't this need a strdup()?

[des]

usr.bin/tftp/main.c

355

uh yes but I meant to move that into setpeer0()

[des]

usr.bin/tftp/main.c

355

Looks like a mismerge on my end accidentally reverted part of D57105

[des]

fix