detect/bytemath: guard right shift against wide counts

Zero the result when a byte_math right shift count reaches 64, the
width of the uint64_t being shifted, so the operation no longer
depends on behavior C11 6.5.7p3 leaves undefined. The left shift case
has done this since 473ca6dcf4; the right shift case was left
unguarded.

DetectByteMathDoMatch() shifted by whatever count it was handed. On
x86_64 the hardware masks the count to its low six bits, so a count of
64 became a shift of 0 and returned the extracted value unchanged
instead of 0. That value is stored in det_ctx->byte_values[] and feeds
any byte_test, isdataat, or content offset later in the signature, so
the signature's verdict follows from an arithmetic result the standard
does not define.

The count reaches the shift from the wire. When byte_math names a
variable for rvalue, DetectEngineContentInspectionInternal() reads it
out of det_ctx->byte_values[] at
detect-engine-content-inspection.c:614, where a preceding byte_extract
stored bytes taken from the payload, so one payload byte of 0x40 sets
the count to 64.

Issue: 8845
pull/16141/head
Jeff Lucovsky 4 weeks ago committed by Victor Julien
parent 389700eca2
commit e5d035fd16

@ -1,4 +1,4 @@
/* Copyright (C) 2020-2022 Open Information Security Foundation
/* Copyright (C) 2020-2026 Open Information Security Foundation
*
* You can copy, redistribute or modify this Program under the terms of
* the GNU General Public License version 2 as published by the Free
@ -189,7 +189,11 @@ int DetectByteMathDoMatch(DetectEngineThreadCtx *det_ctx, const DetectByteMathDa
}
break;
case RightShift:
val >>= rvalue;
if (rvalue < 64) {
val >>= rvalue;
} else {
val = 0;
}
break;
}
@ -999,6 +1003,31 @@ static int DetectByteMathPacket02(void)
PASS;
}
/**
* \test A payload-supplied shift count of 64 or more yields 0 instead of
* shifting a uint64_t by its own width.
*/
static int DetectByteMathPacket03(void)
{
/* byte 0 is the shift count (64), byte 1 the value shifted, byte 2 the
* expected result */
uint8_t buf[] = { 0x40, 0xff, 0x00 };
Packet *p = UTHBuildPacket(buf, sizeof(buf), IPPROTO_UDP);
FAIL_IF_NULL(p);
/* 0xff >> 64 is 0 */
FAIL_IF_NOT(UTHPacketMatchSig(p, "alert udp any any -> any any "
"(byte_extract: 1, 0, shift;"
"byte_math: bytes 1, offset 1, oper >>, rvalue shift, result "
"var;"
"byte_test: 1, =, var, 2;"
"sid:1;)"));
UTHFreePacket(p);
PASS;
}
static int DetectByteMathContext01(void)
{
DetectEngineCtx *de_ctx = NULL;
@ -1071,6 +1100,7 @@ static void DetectByteMathRegisterTests(void)
UtRegisterTest("DetectByteMathParseTest16", DetectByteMathParseTest16);
UtRegisterTest("DetectByteMathPacket01", DetectByteMathPacket01);
UtRegisterTest("DetectByteMathPacket02", DetectByteMathPacket02);
UtRegisterTest("DetectByteMathPacket03", DetectByteMathPacket03);
UtRegisterTest("DetectByteMathContext01", DetectByteMathContext01);
}
#endif /* UNITTESTS */

Loading…
Cancel
Save