Re: [PATCH v2 0/5] fs/iso9660: Fix out-of-bounds read

[Thomas Schmitt]

Hi,

On Wed, 18 Jan 2023 08:23:53 +0000 Lidong Chen <lidong.chen@oracle.com> wrote:
> Thomas also pointed out the issue of the potential endless
> loops by CE. Since the sugguested fix requires a bit more
> investigation, and as Thomas pointed out that it should be
> handled in a separate patch, the fix is not included in this
> this v2 patches set. Because I am not an expert, it would
> be better that someone else can work on it. For the background
> info and the comments, please see this email. The bottom half
> of the email addressed the endless loop issue:
>
> https://www.mail-archive.com/grub-devel@gnu.org/msg35785.html

I had a look at Linux fs/isofs/rock.c about the handling of CE loops.

It works with a hop counter like my current untested proposal for GRUB,
which i made in above mail.
But Linux is very restrictive:

  /* Maximum number of Rock Ridge continuation entries */
  #define RR_MAX_CE_ENTRIES 32
  ...
                ret = -EIO;
                if (++rs->cont_loops >= RR_MAX_CE_ENTRIES)
                        goto out;

So probably my proposed limit of a million is just oversized. But i think
that 32 would be too low.

The Linux limit is bad news for people who want to put larger data blobs
into SUSP controlled blocks, because the size of a continuation area is
also restricted by Linux to a single block of 2048 bytes.
The EIO in case of too much CE entries does not show up in the system log
or on the terminal. Just the file is not listed and not accessible:

  $ wc /mnt/iso/x
  wc: /mnt/iso/x: No such file or directory

I understand from source code and experiments that the actual maximum
number of CE hops in Linux is 31 rather than 32.

libisofs and xorriso are in the process of getting an adjustable curb to
prevent the production of ISO filesystems with files which would not show
up in Linux. I decided for 100,000 hops as hard limit but set the default
to 31.
(SUSP prescribes that entries of unknown type be "ignored and skipped",
which Linux normally does fine. Here it could do better.)


Have a nice day :)

Thomas