Re: [PATCH] ubifs: fix out-of-bounds read in signature length check

----- Ursprüngliche Mail -----
> Von: "Ibrahim Hashimov" <[email protected]>
> An: "richard" <[email protected]>
> CC: "chengzhihao1" <[email protected]>, "Sascha Hauer" <[email protected]>, "linux-mtd"
> <[email protected]>, "linux-kernel" <[email protected]>, "stable" <[email protected]>
> Gesendet: Freitag, 24. Juli 2026 09:43:27
> Betreff: [PATCH] ubifs: fix out-of-bounds read in signature length check

> ubifs_sb_verify_signature() bounds the on-disk ubifs_sig_node->len field
> before handing the signature payload to verify_pkcs7_signature(), but the
> check has the wrong sign:
> 
>	if (le32_to_cpu(signode->len) > snod->len + sizeof(struct ubifs_sig_node))
> 
> The signature bytes start sizeof(struct ubifs_sig_node) (UBIFS_SIG_NODE_SZ,
> 64 bytes) into the node, so the payload is at most
> 
>	snod->len - sizeof(struct ubifs_sig_node)
> 
> bytes long. Adding the header size instead of subtracting it accepts a
> declared length up to 2 * UBIFS_SIG_NODE_SZ larger than the node actually
> holds -- past the end of c->sbuf, which is vmalloc(c->leb_size).
> verify_pkcs7_signature() -> pkcs7_parse_message() -> asn1_ber_decoder()
> is then handed that inflated length and reads beyond the allocation while
> walking the DER headers. The node length comes straight from the mounted
> image, so a crafted signed UBIFS image reaches this via
> ubifs_read_superblock() before the signature is cryptographically checked.
> 
> snod->len is guaranteed to be >= UBIFS_SIG_NODE_SZ by the node scanner
> (c->ranges[UBIFS_SIG_NODE].min_len == UBIFS_SIG_NODE_SZ), so the corrected
> subtraction cannot underflow. Legitimately signed images are unaffected: a
> correct superblock never declares a signature longer than the node it is
> embedded in.
> 
> Fixes: 817aa094842d ("ubifs: support offline signed images")
> Cc: [email protected]
> Signed-off-by: Ibrahim Hashimov <[email protected]>
> Assisted-by: AuditCode-AI:2026.07
> ---
> fs/ubifs/auth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Weinberger <[email protected]>

Thanks,
//richard