Re: ubifs_dump_node must bounds check ubifs_ch->len

[Artem Bityutskiy <dedekind1@gmail.com>]

On Thu, 2014-08-28 at 15:37 -0700, Daniel Mentz wrote:
> I believe that ubifs_dump_node() must bounds check ch->len in the
> UBIFS_DATA_NODE case. It currently does not which resulted in a crash
> on a system. See below.
> 
> This is the source code as it stands today:
> 
> int dlen = le32_to_cpu(ch->len) - UBIFS_DATA_NODE_SZ;
> print_hex_dump(KERN_ERR, "\t", DUMP_PREFIX_OFFSET, 32, 1,
>                                (void *)&dn->data, dlen, 0);

Yes, I agree. This is a bug.

The problem is that this function tries to print the node even though it
may be corrupted and the data are garbage.

So we should assume that any field may contain garbage. I see similar
potential problems.

1.

case UBIFS_DENT_NODE:
case UBIFS_XENT_NODE:

nlen < 0 is not verified.

2.

case UBIFS_DATA_NODE:

you reported - dlen is not validated.

Here we could use 'c->ranges' to validate it before using, see
'ubifs_check_node()' for an example.

3.

case UBIFS_IDX_NODE:

fanout is not validated

4.

case UBIFS_ORPH_NODE:

"n" is not validated.


Will I feel lucky asking you whether you was going to send a patch? :-)

Thanks for the report!

-- 
Best Regards,
Artem Bityutskiy