From c576712ebd463859abe3a0c690f6ab7f2b743e4b Mon Sep 17 00:00:00 2001 From: Gerhard Rieger Date: Mon, 4 Oct 2010 21:04:45 +0200 Subject: [PATCH] fixed a stack overflow vulnerability with long command line args --- CHANGES | 9 +++++++ nestlex.c | 6 ++--- test.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ xioclose.c | 2 +- 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index d77f345..da23391 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,13 @@ +security: + fixed a stack overflow vulnerability that occurred when command + line arguments (whole addresses, host names, file names) were longer + than 512 bytes. + Note that this could only be exploited when an attacker was able to + inject data into socat's command line. + Full credits to Felix Gröbert, Google Security Team, for finding and + reporting this issue + ####################### V 2.0.0-b3: new features: diff --git a/nestlex.c b/nestlex.c index 5230f9b..f8b7c3c 100644 --- a/nestlex.c +++ b/nestlex.c @@ -1,5 +1,5 @@ /* source: nestlex.c */ -/* Copyright Gerhard Rieger 2006-2007 */ +/* Copyright Gerhard Rieger 2006-2010 */ /* Published under the GNU General Public License V.2, see file COPYING */ /* a function for lexical scanning of nested character patterns */ @@ -221,7 +221,7 @@ int nestlex(const char **addr, /* input string; aft points to end token */ } *out++ = c; --*len; - if (len == 0) { + if (*len == 0) { *addr = in; *token = out; return -1; /* output overflow */ @@ -233,7 +233,7 @@ int nestlex(const char **addr, /* input string; aft points to end token */ /* just a simple char */ *out++ = c; --*len; - if (len == 0) { + if (*len == 0) { *addr = in; *token = out; return -1; /* output overflow */ diff --git a/test.sh b/test.sh index d1fbb83..880de5a 100755 --- a/test.sh +++ b/test.sh @@ -10514,6 +10514,75 @@ t TCP # -U exec1 +# socat up to 1.7.1.2 and from 2.0.0-b1 to 2.0.0-b3 had a stack overflow vulnerability that occurred when +# command line arguments (whole addresses, host names, file names) were longer +# than 512 bytes. +NAME=HOSTNAMEOVFL +case "$TESTS" in +*%functions%*|*%bugs%*|*%security%*|*%socket%*|*%$NAME%*) +TEST="$NAME: stack overflow on overly long host name" +# provide a long host name to TCP-CONNECT and check socats exit code +if ! eval $NUMCOND; then :; else +tf="$td/test$N.stdout" +te="$td/test$N.stderr" +tdiff="$td/test$N.diff" +da="test$N $(date) $RANDOM" +# prepare long data - perl might not be installed +rm -f "$td/terst$N.dat" +i=0; while [ $i -lt 64 ]; do echo -n "AAAAAAAAAAAAAAAA" >>"$td/test$N.dat"; i=$((i+1)); done +CMD0="$SOCAT $opts TCP-CONNECT:$(cat "$td/test$N.dat"):$PORT STDIO" +printf "test $F_n $TEST... " $N +$CMD0 &0 2>"${te}0" +rc0=$? +if [ $rc0 -lt 128 ] || [ $rc0 -eq 255 ]; then + $PRINTF "$OK\n" + numOK=$((numOK+1)) +else + $PRINTF "$FAILED\n" + echo "$CMD0" + cat "${te}0" + numFAIL=$((numFAIL+1)) +fi +fi # NUMCOND + ;; +esac +PORT=$((PORT+1)) +N=$((N+1)) + +#set -vx +# socat up to 1.7.1.2 and from 2.0.0-b1 to 2.0.0-b3 had a stack overflow vulnerability that occurred when +# command line arguments (whole addresses, host names, file names) were longer +# than 512 bytes. +NAME=FILENAMEOVFL +case "$TESTS" in +*%functions%*|*%bugs%*|*%security%*|*%openssl%*|*%$NAME%*) +TEST="$NAME: stack overflow on overly long file name" +# provide a 600 bytes long key file option to SSL-CONNECT and check socats exit code +if ! eval $NUMCOND; then :; else +tf="$td/test$N.stdout" +te="$td/test$N.stderr" +tdiff="$td/test$N.diff" +da="test$N $(date) $RANDOM" +i=0; while [ $i -lt 64 ]; do echo -n "AAAAAAAAAAAAAAAA" >>"$td/test$N.dat"; i=$((i+1)); done +CMD0="$SOCAT $opts OPENSSL:localhost:$PORT,key=$(cat "$td/test$N.dat") STDIO" +printf "test $F_n $TEST... " $N +$CMD0 &0 2>"${te}0" +rc0=$? +if [ $rc0 -lt 128 ] || [ $rc0 -eq 255 ]; then + $PRINTF "$OK\n" + numOK=$((numOK+1)) +else + $PRINTF "$FAILED\n" + echo "$CMD0" + cat "${te}0" + numFAIL=$((numFAIL+1)) +fi +fi # NUMCOND + ;; +esac +PORT=$((PORT+1)) +N=$((N+1)) + echo "summary: $((N-1)) tests; $numOK ok, $numFAIL failed, $numCANT could not be performed" diff --git a/xioclose.c b/xioclose.c index af2d6ec..8164f5a 100644 --- a/xioclose.c +++ b/xioclose.c @@ -142,7 +142,7 @@ int xioclose(xiofile_t *file) { } else { result = xioclose1(&file->stream); } - if (xfd->stream.subthread != 0) { + if (xfd->tag != XIO_TAG_INVALID && xfd->stream.subthread != 0) { Pthread_join(xfd->stream.subthread, NULL); } return result;