From 8e6b341f591c864b59f18d11ed14fae23f9de7f1 Mon Sep 17 00:00:00 2001 From: Gerhard Rieger Date: Tue, 13 Oct 2020 20:08:04 +0200 Subject: [PATCH] Fixed possible integer overflow with option -b --- CHANGES | 8 ++++++++ procan.c | 3 +++ socat.c | 4 ++++ test.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index b99de78..6212999 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,12 @@  +Security: + Buffer size option (-b) is internally doubled for CR-CRLF conversion, + but not checked for integer overflow. This could lead to heap based + buffer overflow, assuming the attacker could provide this parameter. + Test: BLKSIZE_INT_OVERFL + Thanks to Lê Hiếu Bùi for reporting this issue and sending an + example exploit. + Testing: test.sh now produces a list of tests that could not be performed for any reason. This helps to analyse these cases. diff --git a/procan.c b/procan.c index 03fe13f..9ebd423 100644 --- a/procan.c +++ b/procan.c @@ -158,6 +158,9 @@ int procan(FILE *outfile) { "virtual memory (kbytes) %24"F_rlim_max"%24"F_rlim_max"\n", rlim.rlim_cur, rlim.rlim_max); } +#endif +#ifdef SIZE_MAX + fprintf(outfile, "SIZE_MAX = %-24lu\n", SIZE_MAX); #endif } diff --git a/socat.c b/socat.c index d65eb65..c772c93 100644 --- a/socat.c +++ b/socat.c @@ -779,6 +779,10 @@ int _socat(void) { #endif /* WITH_FILAN */ /* when converting nl to crnl, size might double */ + if (socat_opts.bufsiz > (SIZE_MAX-1)/2) { + Error2("buffer size option (-b) to big - "F_Zu" (max is "F_Zu")", socat_opts.bufsiz, (SIZE_MAX-1)/2); + socat_opts.bufsiz = (SIZE_MAX-1)/2; + } buff = Malloc(2*socat_opts.bufsiz+1); if (buff == NULL) return -1; diff --git a/test.sh b/test.sh index 6e131f8..6407a89 100755 --- a/test.sh +++ b/test.sh @@ -60,8 +60,8 @@ MISCDELAY=1 if ! [ -x "$SOCAT" ] && ! type $SOCAT >/dev/null 2>&1; then echo "$SOCAT does not exist" >&2; exit 1; fi -[ -z "$PROCAN" ] && PROCAN="./procan" -[ -z "$FILAN" ] && FILAN="./filan" +if [ -z "$PROCAN" ]; then if test -x ./procan; then PROCAN="./procan"; elif ! type procan >/dev/null 2>&1; then PROCAN=${SOCAT%/*}/procan; fi; fi +if [ -z "$FILAN" ]; then if test -x ./filan; then FILAN="./filan"; elif ! type filan >/dev/null 2>&1; then FILAN=${SOCAT%/*}/filan; fi; fi opts="$opt_t $OPTS" export SOCAT_OPTS="$opts" #debug="1" @@ -13461,6 +13461,55 @@ esac N=$((N+1)) +# Test for integer overflow with data transfer block size parameter +NAME=BLKSIZE_INT_OVERFL +case "$TESTS" in +*%$N%*|*%functions%*|*%bugs%*|*%security%*|*%$NAME%*) +TEST="$NAME: integer overflow with buffer size parameter" +# Use a buffer size that would lead to integer overflow +# Test succeeds when Socat terminates with correct error message +if ! eval $NUMCOND; then :; else +tf="$td/test$N.stdout" +te="$td/test$N.stderr" +tdiff="$td/test$N.diff" +dat="$td/test$N.dat" +# calculate the minimal length with integer overflow +BYTES=$($PROCAN |grep size_t |awk '{print($3);}') +case $BYTES in + 2) CHKSIZE=32768 ;; + 4) CHKSIZE=2147483648 ;; + 8) CHKSIZE=9223372036854775808 ;; + 16) CHKSIZE=170141183460469231731687303715884105728 ;; +esac +CMD0="$TRACE $SOCAT $opts -T 1 -b $CHKSIZE /dev/null PIPE" +printf "test $F_n $TEST... " $N +$CMD0 >/dev/null 2>"${te}0" +rc0=$? +if [ $rc0 -eq 0 ]; then + $PRINTF "$FAILED (rc=$rc0)\n" + echo "$CMD0" + cat "${te}0" + numFAIL=$((numFAIL+1)) + listFAIL="$listFAIL $N" +elif [ $rc0 -eq 1 ]; then + if grep -q "buffer size option (-b) to big" "${te}0"; then + $PRINTF "$OK\n" + numOK=$((numOK+1)) + else + $PRINTF "$FAILED (rc=$rc0)\n" + echo "$CMD0" + cat "${te}0" + numFAIL=$((numFAIL+1)) + listFAIL="$listFAIL $N" + fi +fi +fi # NUMCOND + ;; +esac +PORT=$((PORT+1)) +N=$((N+1)) + + ################################################################################## #================================================================================= # here come tests that might affect your systems integrity. Put normal tests