MFC after: 1 week
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 73246 Build 70129: arc lint + arc unit
Event Timeline
| usr.bin/tftp/main.c | ||
|---|---|---|
| 339 | 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. | |
| 341–342 | line is uninitalized (well, NULL) here, and even if not, the getline() is going to discard the existing string. | |
| 428–429 | Same here and further below. | |
| usr.bin/tftp/main.c | ||
|---|---|---|
| 634 | This could never work btw, after makeargv() the value would be in argv[2], not argv[1]. | |
| usr.bin/tftp/main.c | ||
|---|---|---|
| 339 | 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. | |
| usr.bin/tftp/main.c | ||
|---|---|---|
| 339 | 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(). | |
| usr.bin/tftp/main.c | ||
|---|---|---|
| 334 | I think this will break the default case (lport = "tftp") unfortunately: look at how xmitfile() handles port == NULL. | |
| usr.bin/tftp/main.c | ||
|---|---|---|
| 353 | Doesn't this need a strdup()? | |
| usr.bin/tftp/main.c | ||
|---|---|---|
| 353 | uh yes but I meant to move that into setpeer0() | |