From d59de97be98ab70ee38d9efa11d04d8015e23df6 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 22 Apr 2022 09:49:46 +0200 Subject: [PATCH 1/2] net bugfix: potential buffer overrun --- contrib/imhttp/imhttp.c | 4 +++- plugins/imptcp/imptcp.c | 4 +++- runtime/tcps_sess.c | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/contrib/imhttp/imhttp.c b/contrib/imhttp/imhttp.c index f09260b586..95704af985 100644 --- a/contrib/imhttp/imhttp.c +++ b/contrib/imhttp/imhttp.c @@ -487,7 +487,9 @@ processOctetMsgLen(const instanceConf_t *const inst, struct conn_wrkr_s *connWrk connWrkr->parseState.iOctetsRemain = connWrkr->parseState.iOctetsRemain * 10 + ch - '0'; } // temporarily save this character into the message buffer - connWrkr->pMsg[connWrkr->iMsg++] = ch; + if(connWrkr->iMsg + 1 < s_iMaxLine) { + connWrkr->pMsg[connWrkr->iMsg++] = ch; + } } else { const char *remoteAddr = ""; if (connWrkr->propRemoteAddr) { diff --git a/plugins/imptcp/imptcp.c b/plugins/imptcp/imptcp.c index 2df46a236c..c32dec5851 100644 --- a/plugins/imptcp/imptcp.c +++ b/plugins/imptcp/imptcp.c @@ -1107,7 +1107,9 @@ processDataRcvd(ptcpsess_t *const __restrict__ pThis, if(pThis->iOctetsRemain <= 200000000) { pThis->iOctetsRemain = pThis->iOctetsRemain * 10 + c - '0'; } - *(pThis->pMsg + pThis->iMsg++) = c; + if(pThis->iMsg < iMaxLine) { + *(pThis->pMsg + pThis->iMsg++) = c; + } } else { /* done with the octet count, so this must be the SP terminator */ DBGPRINTF("TCP Message with octet-counter, size %d.\n", pThis->iOctetsRemain); prop.GetString(pThis->peerName, &propPeerName, &lenPeerName); diff --git a/runtime/tcps_sess.c b/runtime/tcps_sess.c index 0efa2c23c4..c5442f7638 100644 --- a/runtime/tcps_sess.c +++ b/runtime/tcps_sess.c @@ -390,7 +390,9 @@ processDataRcvd(tcps_sess_t *pThis, if(pThis->iOctetsRemain <= 200000000) { pThis->iOctetsRemain = pThis->iOctetsRemain * 10 + c - '0'; } - *(pThis->pMsg + pThis->iMsg++) = c; + if(pThis->iMsg < iMaxLine) { + *(pThis->pMsg + pThis->iMsg++) = c; + } } else { /* done with the octet count, so this must be the SP terminator */ DBGPRINTF("TCP Message with octet-counter, size %d.\n", pThis->iOctetsRemain); prop.GetString(pThis->fromHost, &propPeerName, &lenPeerName); From 30ccf7cd4c00bfc38c2e0b1461b799b1a412d7fb Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 25 Apr 2022 10:18:46 +0200 Subject: [PATCH 2/2] testbench: new tests for potential buffer overrun --- tests/Makefile.am | 4 ++++ tests/imptcp-octet-framing-too-long-vg.sh | 23 +++++++++++++++++++++++ tests/imtcp-octet-framing-too-long-vg.sh | 23 +++++++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100755 tests/imptcp-octet-framing-too-long-vg.sh create mode 100755 tests/imtcp-octet-framing-too-long-vg.sh diff --git a/tests/Makefile.am b/tests/Makefile.am index 330546890e..805949ec8b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -283,6 +283,7 @@ TESTS += \ allowed-sender-tcp-fail.sh \ allowed-sender-tcp-hostname-ok.sh \ allowed-sender-tcp-hostname-fail.sh \ + imtcp-octet-framing-too-long-vg.sh \ imtcp-discard-truncated-msg.sh \ imtcp-basic.sh \ imtcp-basic-hup.sh \ @@ -1074,6 +1075,7 @@ if ENABLE_IMPTCP # need to be disabled if we do not have this module TESTS += \ manyptcp.sh \ + imptcp-octet-framing-too-long-vg.sh \ imptcp_framing_regex.sh \ imptcp_framing_regex-oversize.sh \ imptcp_large.sh \ @@ -2121,6 +2123,7 @@ EXTRA_DIST= \ mmjsonparse_simple.sh \ mmjsonparse-invalid-containerName.sh \ wtpShutdownAll-assertionFailure.sh \ + imptcp-octet-framing-too-long-vg.sh \ imptcp-oversize-message-display.sh \ imptcp-msg-truncation-on-number.sh \ imptcp-msg-truncation-on-number2.sh \ @@ -2199,6 +2202,7 @@ EXTRA_DIST= \ allowed-sender-tcp-fail.sh \ allowed-sender-tcp-hostname-ok.sh \ allowed-sender-tcp-hostname-fail.sh \ + imtcp-octet-framing-too-long-vg.sh \ imtcp-discard-truncated-msg.sh \ imtcp-basic.sh \ imtcp-basic-hup.sh \ diff --git a/tests/imptcp-octet-framing-too-long-vg.sh b/tests/imptcp-octet-framing-too-long-vg.sh new file mode 100755 index 0000000000..d5b2c9adce --- /dev/null +++ b/tests/imptcp-octet-framing-too-long-vg.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# added 2022-04-25 by RGerhards, released under ASL 2.0 +. ${srcdir:=.}/diag.sh init +generate_conf +add_conf ' +$MaxMessageSize 128 +global(processInternalMessages="on" + oversizemsg.input.mode="accept") +module(load="../plugins/imptcp/.libs/imptcp") +input(type="imptcp" port="0" listenPortFileName="'$RSYSLOG_DYNNAME'.tcpflood_port") + +action(type="omfile" file="'$RSYSLOG_OUT_LOG'") +' +startup_vg +echo "0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000 <120> 2011-03-01T11:22:12Z host tag: this is a way too long message that has to be truncatedtest1 test2 test3 test4 test5 ab" > $RSYSLOG_DYNNAME.inputfile +tcpflood -I $RSYSLOG_DYNNAME.inputfile +shutdown_when_empty +wait_shutdown_vg +check_exit_vg + +# the prime objective is to see if valgrind check is ok, but we also do a quick content check (just in case) +content_check "received oversize message from peer" +exit_test diff --git a/tests/imtcp-octet-framing-too-long-vg.sh b/tests/imtcp-octet-framing-too-long-vg.sh new file mode 100755 index 0000000000..88e8a14fb9 --- /dev/null +++ b/tests/imtcp-octet-framing-too-long-vg.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# added 2022-04-25 by RGerhards, released under ASL 2.0 +. ${srcdir:=.}/diag.sh init +generate_conf +add_conf ' +$MaxMessageSize 128 +global(processInternalMessages="on" + oversizemsg.input.mode="accept") +module(load="../plugins/imtcp/.libs/imtcp") +input(type="imtcp" port="0" listenPortFileName="'$RSYSLOG_DYNNAME'.tcpflood_port") + +action(type="omfile" file="'$RSYSLOG_OUT_LOG'") +' +startup_vg +echo "0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000 <120> 2011-03-01T11:22:12Z host tag: this is a way too long message that has to be truncatedtest1 test2 test3 test4 test5 ab" > $RSYSLOG_DYNNAME.inputfile +tcpflood -I $RSYSLOG_DYNNAME.inputfile +shutdown_when_empty +wait_shutdown_vg +check_exit_vg + +# the prime objective is to see if valgrind check is ok, but we also do a quick content check (just in case) +content_check "received oversize message from peer" +exit_test