Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

detect/bytejump: Restrict post_offset to buffer #9474

Closed
wants to merge 2 commits into from
Closed

detect/bytejump: Restrict post_offset to buffer #9474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2cfbd34
detect/bytejump: Change DoMatch signature to return bool
jlucovsky Sep 8, 2023
f43600b
detect/bytejump: Improve negative post_offset handling.
jlucovsky Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 26 additions & 26 deletions src/detect-bytejump.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,10 @@ static bool DetectBytejumpValidateNbytes(const DetectBytejumpData *data, int32_t
* \param m byte jump sigmatch
* \param payload ptr to the payload
* \param payload_len length of the payload
* \retval 1 match
* \retval 0 no match
* \retval true match
* \retval false no match
*/
int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s,
bool DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s,
const SigMatchCtx *ctx, const uint8_t *payload, uint32_t payload_len, uint16_t flags,
int32_t nbytes, int32_t offset)
{
Expand All @@ -148,7 +148,7 @@ int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s,
int extbytes;

if (payload_len == 0) {
SCReturnInt(0);
SCReturnBool(false);
}

/* Validate the number of bytes we are testing
Expand All @@ -161,7 +161,7 @@ int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s,
SCLogDebug("Invalid byte_jump nbytes "
"seen in byte_jump - %d",
nbytes);
SCReturnInt(0);
SCReturnBool(false);
}
}

Expand All @@ -177,7 +177,7 @@ int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s,

/* No match if there is no relative base */
if (ptr == NULL || len <= 0) {
SCReturnInt(0);
SCReturnBool(false);
}
}
else {
Expand All @@ -190,23 +190,23 @@ int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s,
SCLogDebug("Data not within payload "
"pkt=%p, ptr=%p, len=%d, nbytes=%d",
payload, ptr, len, nbytes);
SCReturnInt(0);
SCReturnBool(false);
}

/* Extract the byte data */
if (flags & DETECT_BYTEJUMP_STRING) {
extbytes = ByteExtractStringUint64(&val, data->base, nbytes, (const char *)ptr);
if(extbytes <= 0) {
SCLogDebug("error extracting %d bytes of string data: %d", nbytes, extbytes);
SCReturnInt(0);
SCReturnBool(false);
}
}
else {
int endianness = (flags & DETECT_BYTEJUMP_LITTLE) ? BYTE_LITTLE_ENDIAN : BYTE_BIG_ENDIAN;
extbytes = ByteExtractUint64(&val, endianness, (uint16_t)nbytes, ptr);
if (extbytes != nbytes) {
SCLogDebug("error extracting %d bytes of numeric data: %d", nbytes, extbytes);
SCReturnInt(0);
SCReturnBool(false);
}
}

Expand All @@ -221,38 +221,37 @@ int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s,
}
val += data->post_offset;

const uint8_t *jumpptr;
/* Calculate the jump location */
if (flags & DETECT_BYTEJUMP_BEGIN) {
SCLogDebug("NEWVAL: payload %p + %" PRIu64, payload, val);
} else if (flags & DETECT_BYTEJUMP_END) {
val = payload_len + val;
SCLogDebug("NEWVAL: payload %p + %" PRIu32 " - %" PRIu64, payload, payload_len, val);
jumpptr = payload;
SCLogDebug("NEWVAL: payload %p + %ld = %p\n", p->payload, val, jumpptr);
} else {
val += (ptr - payload) + extbytes;
SCLogDebug("NEWVAL: ptr %p + %" PRIu64, ptr, val);
SCLogDebug("NEWVAL: ptr %p + %ld = %p\n", ptr, val, jumpptr);
jumpptr = ptr + val + extbytes;
}

/* Validate that the jump location is still in the packet
* \todo Should this validate it is still in the *payload*?
*/
if (val >= payload_len) {
SCLogDebug("Jump location (%" PRIu64 ") is not within "
"payload (%" PRIu32 ")",
val, payload_len);
SCReturnInt(0);
if (jumpptr < payload) {
jumpptr = payload;
SCLogDebug("jump location is not within buffer; resetting to buffer start");
} else if (jumpptr >= (payload + payload_len)) {
SCLogDebug("jump location is past buffer");
SCReturnBool(false);
}

#ifdef DEBUG
if (SCLogDebugEnabled()) {
const uint8_t *sptr = (flags & DETECT_BYTEJUMP_BEGIN) ? payload : ptr;
SCLogDebug("jumping %" PRId64 " bytes from %p (%08x)", val, sptr, (int)(sptr - payload));
}
#endif /* DEBUG */

/* Adjust the detection context to the jump location. */
det_ctx->buffer_offset = val;
det_ctx->buffer_offset = jumpptr - payload;

SCReturnInt(1);
SCReturnBool(true);
}

static int DetectBytejumpMatch(DetectEngineThreadCtx *det_ctx,
Expand Down Expand Up @@ -479,7 +478,8 @@ static DetectBytejumpData *DetectBytejumpParse(
if (*offset == NULL)
goto error;
} else {
if (StringParseInt32(&data->offset, 0, (uint16_t)strlen(args[1]), args[1]) <= 0) {
if (StringParseI32RangeCheck(
&data->offset, 10, (uint16_t)strlen(args[1]), args[1], -65535, 65535) <= 0) {
SCLogError("Malformed offset: %s", optstr);
goto error;
}
Expand Down Expand Up @@ -518,8 +518,8 @@ static DetectBytejumpData *DetectBytejumpParse(
goto error;
}
} else if (strncasecmp("post_offset ", args[i], 12) == 0) {
if (StringParseInt32(&data->post_offset, 10, (uint16_t)strlen(args[i]) - 12,
args[i] + 12) <= 0) {
if (StringParseI32RangeCheck(&data->post_offset, 10, (uint16_t)strlen(args[i]) - 12,
args[i] + 12, -65535, 65535) <= 0) {
SCLogError("Malformed post_offset: %s", optstr);
goto error;
}
Expand Down
12 changes: 3 additions & 9 deletions src/detect-bytejump.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,10 @@ void DetectBytejumpRegister (void);
* \param p pointer to the current packet
* \param m pointer to the sigmatch that we will cast into DetectBytejumpData
*
* \retval -1 error
* \retval 0 no match
* \retval 1 match
*
* \todo The return seems backwards. We should return a non-zero error code.
* One of the error codes is "no match". As-is if someone accidentally
* does: if (DetectBytejumpMatch(...)) { match }, then they catch an
* error as a match.
* \retval false no match
* \retval true
*/
int DetectBytejumpDoMatch(DetectEngineThreadCtx *, const Signature *, const SigMatchCtx *,
bool DetectBytejumpDoMatch(DetectEngineThreadCtx *, const Signature *, const SigMatchCtx *,
const uint8_t *, uint32_t, uint16_t, int32_t, int32_t);

#endif /* __DETECT_BYTEJUMP_H__ */
Expand Down
4 changes: 2 additions & 2 deletions src/detect-engine-content-inspection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,8 @@ uint8_t DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThrea
DETECT_BYTEJUMP_LITTLE: 0);
}

if (DetectBytejumpDoMatch(
det_ctx, s, smd->ctx, buffer, buffer_len, bjflags, nbytes, offset) != 1) {
if (!DetectBytejumpDoMatch(
det_ctx, s, smd->ctx, buffer, buffer_len, bjflags, nbytes, offset)) {
goto no_match;
}

Expand Down