breeze/inetutils
1
0
Fork 0
mirror of https://git.savannah.gnu.org/git/inetutils.git synced 2026-01-12 00:19:39 +08:00
Code Issues Packages Projects Releases Wiki Activity

Buffer overflow in tftp and tftpd.

Browse Source 
Recent Gnu libc and Gcc inject checks for buffer size into strcpy().
These interfere with declarations in <arpa/tftp.h>.  Issue was reported
independently by Mike Gilbert and Ricardo Ribalda Delgado.
This commit is contained in:
Mats Erik Andersson
2020-02-04 11:47:34 +01:00
parent 60d0f2ec4e
commit da919de3f4
4 changed files with 55 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
ChangeLog
View File

@@ -1,3 +1,20 @@
2020-02-04 Mats Erik Andersson <gnu@gisladisker.se>
Buffer overflow in tftp and tftpd.
Recent versions of Gnu libc and Gcc are injecting buffer checks
into strcpy(). Both executables, tftp and tftpd, are effectively
copying into a formally declared `char th_msg[1]', ignorant of
the underlying buffer being of size PKTSIZE.
Problem was reported by Mike Gilbert and Ricardo Ribalda Delgado:
https://lists.gnu.org/archive/html/bug-inetutils/2017-12/msg00001.html
https://lists.gnu.org/archive/html/bug-inetutils/2019-07/msg00002.html
* src/tftp.c (nak): Replace strcpy() by memcpy(), after the
needed calculation of string length.
* src/tftpd.c (nak): Likewise.
* tests/tftp.sh: New compound test with multiple requests.
2020-02-03 Mats Erik Andersson <gnu@gisladisker.se>
whois: AUDA services Australia.

2
src/tftp.c
View File

@@ -1294,8 +1294,8 @@ nak (int error)
pe->e_msg = strerror (error - 100);
tp->th_code = EUNDEF;
}
strcpy (tp->th_msg, pe->e_msg);
length = strlen (pe->e_msg) + 4;
memcpy (tp->th_msg, pe->e_msg, length - 3);
if (trace)
tpacket ("sent", tp, length);
if (sendto (f, ackbuf, length, 0, (struct sockaddr *) &peeraddr,

2
src/tftpd.c
View File

@@ -862,8 +862,8 @@ nak (int error)
pe->e_msg = strerror (error - 100);
tp->th_code = EUNDEF; /* set 'undef' errorcode */
}
strcpy (tp->th_msg, pe->e_msg);
length = strlen (pe->e_msg);
memcpy (tp->th_msg, pe->e_msg, length);
tp->th_msg[length] = '\0';
length += 5;
if (sendto (peer, buf, length, 0, (struct sockaddr *) &from, fromlen) != length)

37
tests/tftp.sh
View File

@@ -386,7 +386,42 @@ get $name" | \
SUCCESSES=`expr $SUCCESSES + 1`
test -z "$VERBOSE" || echo "Successful comparison for $addr/$name." >&2
fi
done
done
# Do a compound test with multiple requests.
# Issue one request for locally renamed file.
rm -f file-small _file-small_ missing-file
EFFORTS=`expr $EFFORTS + 1`
cat <<-EOT |
binary
get file-small
get missing-file
get file-small _file-small_
quit
EOT
eval $TFTP ${VERBOSE:+-v} "$addr" $PORT $bucket
if cmp "$TMPDIR/tftp-test/file-small" file-small 2>/dev/null \
&& test ! -s missing-file \
&& cmp "$TMPDIR/tftp-test/file-small" _file-small_ 2>/dev/null
then
SUCCESSES=`expr $SUCCESSES + 1`
test -z "$VERBOSE" || echo "Successful compound test." >&2
else
echo "Failure during compound test." >&2
# Investigate probable causes.
test -s _file-small_ ||
echo "Third get request failed after file known to be missing." >&2
{ test ! -f missing-file || test -s missing-file ; } &&
echo "The missing file did not appear as empty." >&1
test -s file-small ||
echo "Not even the first request succeeded." >&2
RESULT=1
fi
rm -f file-small _file-small_ missing-file
done
# Test the ability of inetd to reload configuration: