projects / ipfire-2.x.git / blame
| Commit | Line | Data |
|---|---|---|
| 754efda1 MF |
1 | From 294d36df4749e01199ab220d44c170e7db2b0c05 Mon Sep 17 00:00:00 2001 |
| 2 | From: Simon Kelley <simon@thekelleys.org.uk> | |
| 3 | Date: Wed, 6 Jul 2016 21:30:25 +0100 | |
| 4 | Subject: [PATCH] Calculate length of TFTP error reply correctly. | |
| 5 | ||
| 6 | --- | |
| 7 | CHANGELOG | 14 ++++++++++++++ | |
| 8 | src/tftp.c | 7 +++++-- | |
| 9 | 2 files changed, 19 insertions(+), 2 deletions(-) | |
| 10 | ||
| 11 | diff --git a/CHANGELOG b/CHANGELOG | |
| 12 | index 04ff3f0..0559a6f 100644 | |
| 13 | --- a/CHANGELOG | |
| 14 | +++ b/CHANGELOG | |
| 15 | @@ -1,3 +1,17 @@ | |
| 16 | +version 2.77 | |
| 17 | + Calculate the length of TFTP error reply packet | |
| 18 | + correctly. This fixes a problem when the error | |
| 19 | + message in a TFTP packet exceeds the arbitrary | |
| 20 | + limit of 500 characters. The message was correctly | |
| 21 | + truncated, but not the packet length, so | |
| 22 | + extra data was appended. This is a possible | |
| 23 | + security risk, since the extra data comes from | |
| 24 | + a buffer which is also used for DNS, so that | |
| 25 | + previous DNS queries or replies may be leaked. | |
| 26 | + Thanks to Mozilla for funding the security audit | |
| 27 | + which spotted this bug. | |
| 28 | + | |
| 29 | + | |
| 30 | version 2.76 | |
| 31 | Include 0.0.0.0/8 in DNS rebind checks. This range | |
| 32 | translates to hosts on the local network, or, at | |
| 33 | diff --git a/src/tftp.c b/src/tftp.c | |
| 34 | index 5e4a32a..3e1b5c5 100644 | |
| 35 | --- a/src/tftp.c | |
| 36 | +++ b/src/tftp.c | |
| 37 | @@ -652,20 +652,23 @@ static void sanitise(char *buf) | |
| 38 | ||
| 39 | } | |
| 40 | ||
| 41 | +#define MAXMESSAGE 500 /* limit to make packet < 512 bytes and definitely smaller than buffer */ | |
| 42 | static ssize_t tftp_err(int err, char *packet, char *message, char *file) | |
| 43 | { | |
| 44 | struct errmess { | |
| 45 | unsigned short op, err; | |
| 46 | char message[]; | |
| 47 | } *mess = (struct errmess *)packet; | |
| 48 | - ssize_t ret = 4; | |
| 49 | + ssize_t len, ret = 4; | |
| 50 | char *errstr = strerror(errno); | |
| 51 | ||
| 52 | sanitise(file); | |
| 53 | ||
| 54 | mess->op = htons(OP_ERR); | |
| 55 | mess->err = htons(err); | |
| 56 | - ret += (snprintf(mess->message, 500, message, file, errstr) + 1); | |
| 57 | + len = snprintf(mess->message, MAXMESSAGE, message, file, errstr); | |
| 58 | + ret += (len < MAXMESSAGE) ? len + 1 : MAXMESSAGE; /* include terminating zero */ | |
| 59 | + | |
| 60 | my_syslog(MS_TFTP | LOG_ERR, "%s", mess->message); | |
| 61 | ||
| 62 | return ret; | |
| 63 | -- | |
| 64 | 1.7.10.4 | |
| 65 |