All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Thomas Schmitt" <scdbackup@gmx.net>
To: grub-devel@gnu.org
Cc: lidong.chen@oracle.com, fengtao40@huawei.com, yanan@huawei.com,
	daniel.kiper@oracle.com, lichenca2005@gmail.com
Subject: Proposal: fs/iso9660: Prevent skipping CE or ST at start of continuation area
Date: Fri, 16 Dec 2022 10:42:13 +0100	[thread overview]
Message-ID: <19021389617225107434@scdbackup.webframe.org> (raw)
In-Reply-To: <307c2d0fba1b8fb12c0724425b8037d20b7aed06.1671042887.git.lidong.chen@oracle.com>

Hi,

i believe that the problem about CE entries sitting at the start of a SUSP
continuation area could be quite easily solved after patch [2/4] was applied,
which changes the for-loop to a while loop with the entry-advance step at its
end.

So i propose to Lidong Chen to handle this problem by another patch in
the series, which comes after [2/4] (then [2/5]).
Please verify that my thoughts about CE (and additionally about ST) are
correct and consider the following diff, which compiles but is not tested
otherwise.

I agree in advance to changing my "Signed-off-by:" to "Suggested-by:" if
deemed appropriate.

------------------------------------------------------------------------

fs/iso9660: Prevent skipping CE or ST at start of continuation area

If processing of a SUSP CE entry leads to a continuation area which begins
by entry CE or ST, then these entries were skipped without interpretation.
In case of CE this would lead to premature end of processing the SUSP entries
of the file.
In case of ST this could cause following non-SUSP bytes to be interpreted
as SUSP entries.

Signed-off-by: Thomas Schmitt <scdbackup@gmx.net>

--- grub-core/fs/iso9660.c.lidong_chen_patch_2	2022-12-15 11:46:50.654295924 +0100
+++ grub-core/fs/iso9660.c.lidong_chen_patch_2_ce_fix	2022-12-16 10:05:52.990599283 +0100
@@ -336,6 +336,12 @@ grub_iso9660_susp_iterate (grub_fshelp_n
 	    }

 	  entry = (struct grub_iso9660_susp_entry *) sua;
+
+	  if (grub_strncmp ((char *) entry->sig, "CE", 2) == 0
+	      || grub_strncmp ((char *) entry->sig, "ST", 2) == 0)
+	      /* The hook function will not process CE or ST.
+		 Advancing to the next entry would skip them. */
+	      continue;
 	}

       if (hook (entry, hook_arg))

------------------------------------------------------------------------


Have a nice day :)

Thomas



  parent reply	other threads:[~2022-12-16  9:41 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-14 18:55 [PATCH 0/4] fs/iso9660: Fix out-of-bounds read Lidong Chen
2022-12-14 18:55 ` [PATCH 1/4] fs/iso9660: Add check to prevent infinite loop Lidong Chen
2022-12-15 17:52   ` Thomas Schmitt
2022-12-19  8:16     ` Lidong Chen
2022-12-19  9:42       ` Thomas Schmitt
2022-12-14 18:55 ` [PATCH 2/4] fs/iso9660: Prevent read past the end of system use area Lidong Chen
2022-12-15 18:00   ` Thomas Schmitt
2022-12-19  8:39     ` Lidong Chen
2022-12-16  8:54   ` Thomas Schmitt
2022-12-16  9:42   ` Thomas Schmitt [this message]
2022-12-16 12:57     ` Proposal v2: fs/iso9660: Prevent skipping CE or ST at start of continuation area Thomas Schmitt
2022-12-20 21:08       ` Lidong Chen
2023-01-06  5:30       ` Lidong Chen
2023-01-06 16:00         ` Thomas Schmitt
2023-01-09  7:34           ` Lidong Chen
2023-01-09  9:32             ` Thomas Schmitt
2023-01-11 11:54               ` Thomas Schmitt
2023-01-12  5:28                 ` Lidong Chen
2023-01-12  8:45                   ` Thomas Schmitt
2022-12-14 18:55 ` [PATCH 3/4] fs/iso9660: Avoid reading past the entry boundary Lidong Chen
2022-12-15 18:08   ` Thomas Schmitt
2022-12-19  8:42     ` Lidong Chen
2022-12-14 18:55 ` [PATCH 4/4] fs/iso9660: Incorrect check for entry boudary Lidong Chen
2022-12-15 18:20   ` Thomas Schmitt
2022-12-19 21:00     ` Lidong Chen
2022-12-20  9:21       ` Thomas Schmitt
2022-12-14 21:42 ` [PATCH 0/4] fs/iso9660: Fix out-of-bounds read Thomas Schmitt
2022-12-19  8:07   ` Lidong Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=19021389617225107434@scdbackup.webframe.org \
    --to=scdbackup@gmx.net \
    --cc=daniel.kiper@oracle.com \
    --cc=fengtao40@huawei.com \
    --cc=grub-devel@gnu.org \
    --cc=lichenca2005@gmail.com \
    --cc=lidong.chen@oracle.com \
    --cc=yanan@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.