Hi, Thomas
Sorry for the delay, I am also not familiar with ISO format.
But, i have check the cdrkit src-code[1] and syslinux src-code[2]
I think:
(char *) entry < (char *) sua + sua_size - 3 && entry->len > 0
is ok, or:
(char *) entry <= (char *) sua + sua_size - 4 && entry->len > 0
[1]:https://github.com/Distrotech/cdrkit/blob/distrortech-cdrkit/genisoimage/diag/isoinfo.c#L373
[2]:https://github.com/geneC/syslinux/blob/master/core/fs/iso9660/susp_rr.c#L145
On 2022/11/24 23:16, Thomas Schmitt wrote:
> Hi,
>
> (Again i Cc t.feng in the hope that the review is not finished yet. :))
>
> Daniel Kiper wrote:
>> I am not an ISO format expert but your thinking LGTM.
>
> So you agree that "3" is really the right number if any remaining bytes
> fewer than 4 shall be ignored ?
> (I don't trust myself, although i made an example with finger counting.)
>
>
>> could you send a patch fixing this issue?
>
> This should be possible. But how to test ?
>
> Normal ISOs made with GNU/Linux will cause (entry == sua + sua_size) at
> the end of the SUSP data. So provoking the problem and checking whether
> it is solved will need a hacked ISO.
> I will think about creating such an ISO by help of xorriso and dd.
>
>
> While exploring the SUSP/RRIP entry types which are interpreted by GRUB,
> i got to more credulence towards the ISO producer.
> E.g. in grub-core/fs/iso9660.c line 404 ff.
>
> /* The 2nd data byte stored how many bytes are skipped every time
> to get to the SUA (System Usage Area). */
> data->susp_skip = entry->data[2];
>
> This is a memory fault if (sua_size < 7). I see no check between
> sua = grub_malloc (sua_size);
> and the read access to entry->data[2].
>
> Another example:
> grub_iso9660_susp_iterate() calls its parameter hook() without checking
> that the alleged entry size is within the range of sua_size. The hook()
> function does not get sua_size to check on its own.
>
> In general:
> How mistrusting should GRUB be towards the bytes in the filesystem ?
>
>
> Have a nice day :)
>
> Thomas
>
> .
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
