mirror of
https://github.com/systemd/systemd.git
synced 2026-08-12 22:17:19 +00:00
sd-bus: add depth limit to message_skip_fields() to prevent stack overflow (#42164)
`message_skip_fields()` recursively processes D-Bus variant types in message header fields with no depth limit. A crafted message with deeply nested variants can cause unbounded recursion and overflow the stack. Add a `depth` parameter checked against `BUS_CONTAINER_DEPTH` (128), matching the limit already enforced by the public `sd_bus_message_skip()` API. All recursive call sites pass `depth + 1`, and the top-level caller in `message_parse_fields()` passes `0`.
This commit is contained in:
@@ -3904,7 +3904,8 @@ static int message_skip_fields(
|
||||
sd_bus_message *m,
|
||||
size_t *ri,
|
||||
uint32_t array_size,
|
||||
const char **signature) {
|
||||
const char **signature,
|
||||
unsigned depth) {
|
||||
|
||||
size_t original_index;
|
||||
int r;
|
||||
@@ -3913,6 +3914,9 @@ static int message_skip_fields(
|
||||
assert(ri);
|
||||
assert(signature);
|
||||
|
||||
if (depth >= BUS_CONTAINER_DEPTH)
|
||||
return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Maximum container nesting depth reached, refusing.");
|
||||
|
||||
original_index = *ri;
|
||||
|
||||
for (;;) {
|
||||
@@ -3993,7 +3997,7 @@ static int message_skip_fields(
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
r = message_skip_fields(m, ri, nas, (const char**) &s);
|
||||
r = message_skip_fields(m, ri, nas, (const char**) &s, depth + 1);
|
||||
if (r < 0)
|
||||
return r;
|
||||
}
|
||||
@@ -4007,7 +4011,7 @@ static int message_skip_fields(
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
r = message_skip_fields(m, ri, UINT32_MAX, &s);
|
||||
r = message_skip_fields(m, ri, UINT32_MAX, &s, depth + 1);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
@@ -4025,7 +4029,7 @@ static int message_skip_fields(
|
||||
strncpy(sig, *signature + 1, l);
|
||||
sig[l] = '\0';
|
||||
|
||||
r = message_skip_fields(m, ri, UINT32_MAX, (const char**) &s);
|
||||
r = message_skip_fields(m, ri, UINT32_MAX, (const char**) &s, depth + 1);
|
||||
if (r < 0)
|
||||
return r;
|
||||
}
|
||||
@@ -4198,7 +4202,7 @@ static int message_parse_fields(sd_bus_message *m, bool got_ctrunc) {
|
||||
break;
|
||||
|
||||
default:
|
||||
r = message_skip_fields(m, &ri, UINT32_MAX, &signature);
|
||||
r = message_skip_fields(m, &ri, UINT32_MAX, &signature, 0);
|
||||
}
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
@@ -251,6 +251,58 @@ static void test_bus_fds_truncated(void) {
|
||||
log_info("All fd truncation tests passed");
|
||||
}
|
||||
|
||||
static void test_bus_nested_variant_depth_limit(void) {
|
||||
/* Craft a raw D-Bus message with an unknown header field whose value is a variant
|
||||
* containing a variant containing a variant... nested beyond BUS_CONTAINER_DEPTH.
|
||||
* Without the depth limit in message_skip_fields(), this causes unbounded recursion
|
||||
* and stack overflow. With the fix, it should be rejected with -EBADMSG. */
|
||||
|
||||
_cleanup_(sd_bus_unrefp) sd_bus *bus = NULL;
|
||||
_cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
|
||||
const unsigned depth = BUS_CONTAINER_DEPTH + 1; /* one past the limit */
|
||||
|
||||
/* Each nesting level in the fields area is: 1 byte sig_len + 1 byte 'v' + 1 byte NUL = 3 bytes.
|
||||
* The innermost level has sig_len=1, 'u', NUL, then 4 bytes for the uint32 value.
|
||||
* The field header is: 1 byte field_code + 1 byte sig_len + 1 byte 'v' + 1 byte NUL = 4 bytes. */
|
||||
size_t fields_size = 4 + (depth * 3) + 4; /* field header + nested variant sigs + uint32 */
|
||||
size_t padded_fields = ALIGN8(fields_size);
|
||||
size_t total = sizeof(BusMessageHeader) + padded_fields;
|
||||
|
||||
_cleanup_free_ void *buf = ASSERT_PTR(malloc0(total));
|
||||
|
||||
BusMessageHeader *h = buf;
|
||||
*h = (BusMessageHeader) {
|
||||
.endian = BUS_NATIVE_ENDIAN,
|
||||
.type = SD_BUS_MESSAGE_METHOD_CALL,
|
||||
.version = 1,
|
||||
.serial = 1,
|
||||
.fields_size = (uint32_t) fields_size,
|
||||
};
|
||||
|
||||
uint8_t *p = (uint8_t *) buf + sizeof(BusMessageHeader);
|
||||
|
||||
/* Unknown field code (triggers default: in message_parse_fields) */
|
||||
*p++ = 0xFF;
|
||||
/* Field signature: variant */
|
||||
*p++ = 1; /* sig length */
|
||||
*p++ = 'v';
|
||||
*p++ = '\0';
|
||||
|
||||
/* Nested variant signatures: each level declares its content is another variant */
|
||||
for (unsigned i = 0; i < depth; i++) {
|
||||
*p++ = 1; /* sig length */
|
||||
*p++ = 'v';
|
||||
*p++ = '\0';
|
||||
}
|
||||
|
||||
/* Innermost value: a uint32 */
|
||||
memset(p, 0, 4);
|
||||
|
||||
ASSERT_OK(sd_bus_new(&bus));
|
||||
|
||||
ASSERT_ERROR(bus_message_from_malloc(bus, buf, total, NULL, 0, false, NULL, &m), EBADMSG);
|
||||
}
|
||||
|
||||
static void test_bus_label_escape(void) {
|
||||
test_bus_label_escape_one("foo123bar", "foo123bar");
|
||||
test_bus_label_escape_one("foo.bar", "foo_2ebar");
|
||||
@@ -561,6 +613,7 @@ int main(int argc, char *argv[]) {
|
||||
test_bus_path_encode_unique();
|
||||
test_bus_path_encode_many();
|
||||
test_bus_fds_truncated();
|
||||
test_bus_nested_variant_depth_limit();
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user