[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 4/5] fs/iso9660: Incorrect check for entry boundary
From: |
Thomas Schmitt |
Subject: |
Re: [PATCH v2 4/5] fs/iso9660: Incorrect check for entry boundary |
Date: |
Wed, 18 Jan 2023 17:17:35 +0100 |
Hi,
On Wed, 18 Jan 2023 08:23:57 +0000 Lidong Chen <lidong.chen@oracle.com> wrote:
> An SL entry consists of the entry info and the component area.
> The entry info should take up 5 bytes instead of sizeof (*entry).
> The area after the first 5 bytes is the component area. It is
> incorrect to use the sizeof (*entry) to check the entry boundary.
>
> Signed-off-by: Lidong Chen <lidong.chen@oracle.com>
> Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>
> ---
> grub-core/fs/iso9660.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c
> index c6d65fc22..ca45b3424 100644
> --- a/grub-core/fs/iso9660.c
> +++ b/grub-core/fs/iso9660.c
> @@ -663,10 +663,23 @@ susp_iterate_dir (struct grub_iso9660_susp_entry *entry,
> else if (grub_strncmp ("SL", (char *) entry->sig, 2) == 0)
> {
> unsigned int pos = 1;
> + unsigned int csize;
>
> - /* The symlink is not stored as a POSIX symlink, translate it. */
> - while (pos + sizeof (*entry) < entry->len)
> + /* The symlink is not stored as a POSIX symlink, translate it. */
> + while ((pos + GRUB_ISO9660_SUSP_HEADER_SZ + 1) < entry->len)
> {
> + /*
> + * entry->len is GRUB_ISO9660_SUSP_HEADER_SZ + 1 (the FLAGS)
> + * + length of the "Component Area". The length of a component
> + * record is 2 (pos and pos + 1) plus the "Component Content",
> + * of which starts at pos + 2. entry->data[pos] is the
> + * 'Component Flags'; entry->data[pos + 1] is the length
> + * of the component.
> + */
> + csize = entry->data[pos + 1] + 2;
> + if (GRUB_ISO9660_SUSP_HEADER_SZ + 1 + csize > entry->len)
> + break;
> +
> /* The current position is the `Component Flag'. */
> switch (entry->data[pos] & 30)
> {
> --
> 2.35.1
Reviewed-by: Thomas Schmitt <scdbackup@gmx.net>
Most of my initial objections towards patch 4 were wrong. What remained
is taken into respect now.
Have a nice day :)
Thomas
- [PATCH v2 0/5] fs/iso9660: Fix out-of-bounds read, Lidong Chen, 2023/01/18
- [PATCH v2 4/5] fs/iso9660: Incorrect check for entry boundary, Lidong Chen, 2023/01/18
- Re: [PATCH v2 4/5] fs/iso9660: Incorrect check for entry boundary,
Thomas Schmitt <=
- [PATCH v2 3/5] fs/iso9660: Avoid reading past the entry boundary, Lidong Chen, 2023/01/18
- [PATCH v2 1/5] fs/iso9660: Add check to prevent infinite loop, Lidong Chen, 2023/01/18
- [PATCH v2 5/5] fs/iso9660: Prevent skipping CE or ST at start of continuation area, Lidong Chen, 2023/01/18
- [PATCH v2 2/5] fs/iso9660: Prevent read past the end of system use area, Lidong Chen, 2023/01/18