From 04ad6d52751c9ef6b1e42042a83bef6b240f4b22 Mon Sep 17 00:00:00 2001 From: Gerhard Rieger Date: Tue, 21 Jan 2014 19:26:14 +0100 Subject: [PATCH] Red Hat issue 1022063: out-of-range shifts on net mask bits --- CHANGES | 2 ++ test.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ xio-ip4.c | 18 +++++++++++++++--- xio-ip6.c | 27 +++++++++++++++++++-------- 4 files changed, 85 insertions(+), 11 deletions(-) diff --git a/CHANGES b/CHANGES index 4b4300d..fde88c3 100644 --- a/CHANGES +++ b/CHANGES @@ -30,6 +30,8 @@ corrections: On big endian platforms with type long >32bit the range option applied a bad base address. Thanks to hejia hejia for reporting and fixing this bug. + Red Hat issue 1022063: out-of-range shifts on net mask bits + Red Hat issue 1022062: strcpy misuse in xiosetsockaddrenv_ip4() Red Hat issue 1022048: strncpy hardening: corrected suspicious strncpy() diff --git a/test.sh b/test.sh index aaca04d..5afa61a 100755 --- a/test.sh +++ b/test.sh @@ -11249,6 +11249,55 @@ PORT=$((PORT+1)) N=$((N+1)) +# socat up to version 1.7.2.3 +# had a bug that converted a bit mask of 0 internally to 0xffffffff +NAME=TCP4RANGE_0BITS +case "$TESTS" in +*%functions%*|*%tcp%*|*%tcp4%*|*%ip4%*|*%range%*|*%$NAME%*) +TEST="$NAME: correct evaluation of range mask 0" +if ! eval $NUMCOND; then :; +elif [ -z "$SECONDADDR" ]; then + # we need access to a second addresses + $PRINTF "test $F_n $TEST... ${YELLOW}need a second IPv4 address${NORMAL}\n" $N + numCANT=$((numCANT+1)) +else +tf="$td/test$N.stdout" +tdiff="$td/test$N.diff" +da="test$N $(date) $RANDOM" +#testserversec "$N" "$TEST" "$opts -s" "tcp4-l:$PORT,reuseaddr,fork,retry=1" "" "range=$SECONDADDR/32" "tcp4:127.0.0.1:$PORT" 4 tcp $PORT 0 +CMD0="$SOCAT $opts TCP4-LISTEN:$PORT,reuseaddr,range=127.0.0.1/0 CREATE:$tf" +CMD1="$SOCAT $opts - TCP4-CONNECT:$SECONDADDR:$PORT,bind=$SECONDADDR" +printf "test $F_n $TEST... " $N +$CMD0 2>"${te}0" & +pid0=$! +waittcp4port $PORT 1 +echo "$da" |$CMD1 >"${tf}1" 2>"${te}1" +rc1=$? +kill $pid0 2>/dev/null; wait +if [ $rc1 != 0 ]; then + $PRINTF "${YELLOW}invocation failed${NORMAL}\n" + numCANT=$((numCANT+1)) +elif ! [ -f "$tf" ]; then + $PRINTF "$FAILED\n" + echo "$CMD0 &" + echo "$CMD1" + cat "${te}0" + cat "${te}1" + numFAIL=$((numFAIL+1)) +elif ! echo "$da" |diff - "$tf" >"$tdiff"; then + $PRINTF "${YELLOW}diff failed${NORMAL}\n" + numCANT=$((numCANT+1)) +else + $PRINTF "$OK\n" + numOK=$((numOK+1)) +fi + +fi ;; # $SECONDADDR, NUMCOND +esac +PORT=$((PORT+1)) +N=$((N+1)) + + ############################################################################### # here come tests that might affect your systems integrity. Put normal tests # before this paragraph. diff --git a/xio-ip4.c b/xio-ip4.c index 0e41d3f..73d7d7a 100644 --- a/xio-ip4.c +++ b/xio-ip4.c @@ -20,7 +20,7 @@ int xioparsenetwork_ip4(const char *rangename, struct xiorange *range) { struct in_addr *netmask_in = &range->netmask.ip4.sin_addr; char *rangename1; /* a copy of rangename with writing allowed */ char *delimpos; /* absolute address of delimiter */ - int bits; + unsigned int bits; /* netmask bits */ if ((rangename1 = strdup(rangename)) == NULL) { Error1("strdup(\"%s\"): out of memory", rangename); @@ -28,8 +28,20 @@ int xioparsenetwork_ip4(const char *rangename, struct xiorange *range) { } if (delimpos = strchr(rangename1, '/')) { - bits = strtoul(delimpos+1, NULL, 10); - netmask_in->s_addr = htonl((0xffffffff << (32-bits))); + char *endptr; + bits = strtoul(delimpos+1, &endptr, 10); + if (! ((*(delimpos+1) != '\0') && (*endptr == '\0'))) { + Error1("not a valid netmask in \"%s\"", rangename); + bits = 32; /* most secure selection */ + } else if (bits > 32) { + Error1("netmask \"%s\" is too large", rangename); + bits = 32; + } + if (bits <= 0) { + netmask_in->s_addr = 0; + } else { + netmask_in->s_addr = htonl((0xffffffff << (32-bits))); + } } else if (delimpos = strchr(rangename1, ':')) { if ((maskaddr = Gethostbyname(delimpos+1)) == NULL) { /* note: cast is req on AIX: */ diff --git a/xio-ip6.c b/xio-ip6.c index 2644c00..60218ac 100644 --- a/xio-ip6.c +++ b/xio-ip6.c @@ -78,7 +78,8 @@ const struct optdesc opt_ipv6_recvpathmtu = { "ipv6-recvpathmtu", "recvpathmtu", int xioparsenetwork_ip6(const char *rangename, struct xiorange *range) { char *delimpos; /* absolute address of delimiter */ size_t delimind; /* index of delimiter in string */ - int bits; + unsigned int bits; /* netmask bits */ + char *endptr; char *baseaddr; union sockaddr_union sockaddr; socklen_t sockaddrlen = sizeof(sockaddr); @@ -112,22 +113,32 @@ int xioparsenetwork_ip6(const char *rangename, struct xiorange *range) { rangeaddr->u6_addr32[1] = nameaddr->u6_addr32[1]; rangeaddr->u6_addr32[2] = nameaddr->u6_addr32[2]; rangeaddr->u6_addr32[3] = nameaddr->u6_addr32[3]; - bits = strtoul(delimpos+1, NULL, 10); - if (bits > 128) { - Error1("invalid number of mask bits %u", bits); - return STAT_NORETRY; + bits = strtoul(delimpos+1, &endptr, 10); + if (! ((*(delimpos+1) != '\0') && (*endptr == '\0'))) { + Error1("not a valid netmask in \"%s\"", rangename); + bits = 128; /* most secure selection */ + } else if (bits > 128) { + Error1("netmask \"%s\" is too large", rangename); + bits = 128; } - if (bits < 32) { + + /* I am starting to dislike C...uint32_t << 32 is undefined... */ + if (bits == 0) { + rangemask->u6_addr32[0] = 0; + rangemask->u6_addr32[1] = 0; + rangemask->u6_addr32[2] = 0; + rangemask->u6_addr32[3] = 0; + } else if (bits <= 32) { rangemask->u6_addr32[0] = htonl(0xffffffff << (32-bits)); rangemask->u6_addr32[1] = 0; rangemask->u6_addr32[2] = 0; rangemask->u6_addr32[3] = 0; - } else if (bits < 64) { + } else if (bits <= 64) { rangemask->u6_addr32[0] = 0xffffffff; rangemask->u6_addr32[1] = htonl(0xffffffff << (64-bits)); rangemask->u6_addr32[2] = 0; rangemask->u6_addr32[3] = 0; - } else if (bits < 96) { + } else if (bits <= 96) { rangemask->u6_addr32[0] = 0xffffffff; rangemask->u6_addr32[1] = 0xffffffff; rangemask->u6_addr32[2] = htonl(0xffffffff << (96-bits));