Re: Possible memory fault in fs/iso9660 (correction)

["Thomas Schmitt" <scdbackup@gmx.net>]

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