GithubStar/open62541
1
0
Fork 0
mirror of https://github.com/open62541/open62541.git synced 2026-10-17 04:42:09 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(core): validate ExtensionObject body length against decoded byte count

Browse Source 
The binary decoder for ExtensionObject and Variant-of-ExtensionObject
skipped the on-wire body-length field without verifying that the inner
type decoder consumed exactly that many bytes.  A crafted message could
cause the decoder's byte-position to diverge from the length-field's
authoritative interpretation (OPC UA Part 6 §5.2.2.15), creating a
decoder-vs-IDS split-view: any inline network monitor parsing per the
spec observes a different logical message than the server processes.

Two call sites are fixed (Variant-of-EO-array was not yet present in 1.3):

1. ExtensionObject_decodeBinaryContent – pre-auth reachable via
   ActivateSessionRequest.UserIdentityToken.

2. Variant_decodeBinaryUnwrapExtensionObject – pre-auth reachable via
   Variants in OpenSecureChannelRequest / ActivateSessionRequest.

At every site, the body length is now read into a local variable, an
expected-end pointer is computed, and a UA_STATUSCODE_BADDECODINGERROR
is returned if the inner decoder leaves the position anywhere other than
expected_end.

See: open62541-SA-2026-0018

Thanks to Chuan Wei [SCU] for the responsible disclosure.
This commit is contained in:
Niels Beier
2026-06-11 11:43:07 +02:00
committed by Julius Pfrommer
parent 736102e8b2
commit 57142d6b4c
1 changed files with 24 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ua_types_encoding_binary.c
+24 -6
View File
@@ -894,13 +894,21 @@ ExtensionObject_decodeBinaryContent(UA_ExtensionObject *dst, const UA_NodeId *ty
dst->content.decoded.data = UA_new(type);
UA_CHECK_MEM(dst->content.decoded.data, return UA_STATUSCODE_BADOUTOFMEMORY);
/* Jump over the length field (TODO: check if the decoded length matches) */
ctx->pos += 4;
/* Read the length field and validate that the inner decoder consumes exactly
* that many bytes, closing a decoder-vs-IDS split-view channel. */
u32 member_length = 0;
status ret = DECODE_DIRECT(&member_length, UInt32);
UA_CHECK_STATUS(ret, return ret);
UA_CHECK(ctx->pos + member_length <= ctx->end, return UA_STATUSCODE_BADDECODINGERROR);
const u8 *expected_end = ctx->pos + member_length;
/* Decode */
dst->encoding = UA_EXTENSIONOBJECT_DECODED;
dst->content.decoded.type = type;
return decodeBinaryJumpTable[type->typeKind](dst->content.decoded.data, type, ctx);
ret = decodeBinaryJumpTable[type->typeKind](dst->content.decoded.data, type, ctx);
if(ret == UA_STATUSCODE_GOOD && ctx->pos != expected_end)
return UA_STATUSCODE_BADDECODINGERROR;
return ret;
}
DECODE_BINARY(ExtensionObject) {
@@ -1057,10 +1065,17 @@ Variant_decodeBinaryUnwrapExtensionObject(UA_Variant *dst, Ctx *ctx) {
UA_CHECK_STATUS(ret, UA_NodeId_clear(&typeId); return ret);
/* Search for the datatype. Default to ExtensionObject. */
const u8 *expected_end = NULL;
if(encoding == UA_EXTENSIONOBJECT_ENCODED_BYTESTRING &&
(dst->type = UA_findDataTypeByBinaryInternal(&typeId, ctx)) != NULL) {
/* Jump over the length field (TODO: check if length matches) */
ctx->pos += 4;
/* Read the length field and validate that the inner decoder consumes
* exactly that many bytes, closing a decoder-vs-IDS split-view channel. */
u32 member_length = 0;
ret = DECODE_DIRECT(&member_length, UInt32);
UA_CHECK_STATUS(ret, UA_NodeId_clear(&typeId); return ret);
UA_CHECK(ctx->pos + member_length <= ctx->end,
UA_NodeId_clear(&typeId); return UA_STATUSCODE_BADDECODINGERROR);
expected_end = ctx->pos + member_length;
} else {
/* Reset and decode as ExtensionObject */
dst->type = &UA_TYPES[UA_TYPES_EXTENSIONOBJECT];
@@ -1073,7 +1088,10 @@ Variant_decodeBinaryUnwrapExtensionObject(UA_Variant *dst, Ctx *ctx) {
UA_CHECK_MEM(dst->data, return UA_STATUSCODE_BADOUTOFMEMORY);
/* Decode the content */
return decodeBinaryJumpTable[dst->type->typeKind](dst->data, dst->type, ctx);
ret = decodeBinaryJumpTable[dst->type->typeKind](dst->data, dst->type, ctx);
if(ret == UA_STATUSCODE_GOOD && expected_end != NULL && ctx->pos != expected_end)
return UA_STATUSCODE_BADDECODINGERROR;
return ret;
}
/* The resulting variant always has the storagetype UA_VARIANT_DATA. */