* [PATCH v2 0/1] fs/udf: Fix out of bounds access @ 2023-06-07 1:31 Lidong Chen 2023-06-07 1:31 ` [PATCH v2 1/1] " Lidong Chen 0 siblings, 1 reply; 4+ messages in thread From: Lidong Chen @ 2023-06-07 1:31 UTC (permalink / raw) To: grub-devel; +Cc: daniel.kiper, darren.kenny, lidong.chen Thanks Daniel and Darren for reviewing the changes. This v2 addressed Darren's comments. Lidong Chen (1): fs/udf: Fix out of bounds access grub-core/fs/udf.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) -- 2.39.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] fs/udf: Fix out of bounds access 2023-06-07 1:31 [PATCH v2 0/1] fs/udf: Fix out of bounds access Lidong Chen @ 2023-06-07 1:31 ` Lidong Chen 2023-06-07 19:26 ` Darren Kenny 0 siblings, 1 reply; 4+ messages in thread From: Lidong Chen @ 2023-06-07 1:31 UTC (permalink / raw) To: grub-devel; +Cc: daniel.kiper, darren.kenny, lidong.chen Implemented a boundary check before advancing the allocation descriptors pointer. Signed-off-by: Lidong Chen <lidong.chen@oracle.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> --- grub-core/fs/udf.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c index 7679ea309..58884d2ba 100644 --- a/grub-core/fs/udf.c +++ b/grub-core/fs/udf.c @@ -114,6 +114,10 @@ GRUB_MOD_LICENSE ("GPLv3+"); #define GRUB_UDF_PARTMAP_TYPE_1 1 #define GRUB_UDF_PARTMAP_TYPE_2 2 +#define GRUB_UDF_INVALID_STRUCT_PTR(_ptr, _struct) \ + ((char *) (_ptr) >= end_ptr || \ + ((grub_ssize_t)(end_ptr - (char*)(_ptr)) < (grub_ssize_t)sizeof(_struct))) + struct grub_udf_lb_addr { grub_uint32_t block_num; @@ -458,6 +462,7 @@ grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) char *ptr; grub_ssize_t len; grub_disk_addr_t filebytes; + char *end_ptr; switch (U16 (node->block.fe.tag.tag_ident)) { @@ -476,9 +481,17 @@ grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) return 0; } + end_ptr = (char *) node + get_fshelp_size (node->data); + if ((U16 (node->block.fe.icbtag.flags) & GRUB_UDF_ICBTAG_FLAG_AD_MASK) == GRUB_UDF_ICBTAG_FLAG_AD_SHORT) { + if (GRUB_UDF_INVALID_STRUCT_PTR(ptr, struct grub_udf_short_ad)) + { + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); + return 0; + } + struct grub_udf_short_ad *ad = (struct grub_udf_short_ad *) ptr; filebytes = fileblock * U32 (node->data->lvd.bsize); @@ -542,10 +555,22 @@ grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) filebytes -= adlen; ad++; len -= sizeof (struct grub_udf_short_ad); + + if (GRUB_UDF_INVALID_STRUCT_PTR(ad, struct grub_udf_short_ad)) + { + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); + return 0; + } } } else { + if (GRUB_UDF_INVALID_STRUCT_PTR(ptr, struct grub_udf_long_ad)) + { + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); + return 0; + } + struct grub_udf_long_ad *ad = (struct grub_udf_long_ad *) ptr; filebytes = fileblock * U32 (node->data->lvd.bsize); @@ -611,6 +636,12 @@ grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) filebytes -= adlen; ad++; len -= sizeof (struct grub_udf_long_ad); + + if (GRUB_UDF_INVALID_STRUCT_PTR(ad, struct grub_udf_long_ad)) + { + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); + return 0; + } } } @@ -630,6 +661,7 @@ grub_udf_read_file (grub_fshelp_node_t node, case GRUB_UDF_ICBTAG_FLAG_AD_IN_ICB: { char *ptr; + char *end_ptr = (char *) node + get_fshelp_size (node->data); ptr = ((U16 (node->block.fe.tag.tag_ident) == GRUB_UDF_TAG_IDENT_FE) ? ((char *) &node->block.fe.ext_attr[0] @@ -637,6 +669,12 @@ grub_udf_read_file (grub_fshelp_node_t node, ((char *) &node->block.efe.ext_attr[0] + U32 (node->block.efe.ext_attr_length))); + if ((ptr + pos + len) > end_ptr) + { + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); + return 0; + } + grub_memcpy (buf, ptr + pos, len); return len; -- 2.39.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] fs/udf: Fix out of bounds access 2023-06-07 1:31 ` [PATCH v2 1/1] " Lidong Chen @ 2023-06-07 19:26 ` Darren Kenny 2023-06-12 14:16 ` Daniel Kiper 0 siblings, 1 reply; 4+ messages in thread From: Darren Kenny @ 2023-06-07 19:26 UTC (permalink / raw) To: Lidong Chen, grub-devel; +Cc: daniel.kiper, lidong.chen Hi Li,, LGTM! Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Thanks, Darren. On Wednesday, 2023-06-07 at 01:31:06 UTC, Lidong Chen wrote: > Implemented a boundary check before advancing the allocation > descriptors pointer. > > Signed-off-by: Lidong Chen <lidong.chen@oracle.com> > Reviewed-by: Darren Kenny <darren.kenny@oracle.com> > Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> > --- > grub-core/fs/udf.c | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c > index 7679ea309..58884d2ba 100644 > --- a/grub-core/fs/udf.c > +++ b/grub-core/fs/udf.c > @@ -114,6 +114,10 @@ GRUB_MOD_LICENSE ("GPLv3+"); > #define GRUB_UDF_PARTMAP_TYPE_1 1 > #define GRUB_UDF_PARTMAP_TYPE_2 2 > > +#define GRUB_UDF_INVALID_STRUCT_PTR(_ptr, _struct) \ > + ((char *) (_ptr) >= end_ptr || \ > + ((grub_ssize_t)(end_ptr - (char*)(_ptr)) < (grub_ssize_t)sizeof(_struct))) > + > struct grub_udf_lb_addr > { > grub_uint32_t block_num; > @@ -458,6 +462,7 @@ grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) > char *ptr; > grub_ssize_t len; > grub_disk_addr_t filebytes; > + char *end_ptr; > > switch (U16 (node->block.fe.tag.tag_ident)) > { > @@ -476,9 +481,17 @@ grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) > return 0; > } > > + end_ptr = (char *) node + get_fshelp_size (node->data); > + > if ((U16 (node->block.fe.icbtag.flags) & GRUB_UDF_ICBTAG_FLAG_AD_MASK) > == GRUB_UDF_ICBTAG_FLAG_AD_SHORT) > { > + if (GRUB_UDF_INVALID_STRUCT_PTR(ptr, struct grub_udf_short_ad)) > + { > + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); > + return 0; > + } > + > struct grub_udf_short_ad *ad = (struct grub_udf_short_ad *) ptr; > > filebytes = fileblock * U32 (node->data->lvd.bsize); > @@ -542,10 +555,22 @@ grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) > filebytes -= adlen; > ad++; > len -= sizeof (struct grub_udf_short_ad); > + > + if (GRUB_UDF_INVALID_STRUCT_PTR(ad, struct grub_udf_short_ad)) > + { > + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); > + return 0; > + } > } > } > else > { > + if (GRUB_UDF_INVALID_STRUCT_PTR(ptr, struct grub_udf_long_ad)) > + { > + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); > + return 0; > + } > + > struct grub_udf_long_ad *ad = (struct grub_udf_long_ad *) ptr; > > filebytes = fileblock * U32 (node->data->lvd.bsize); > @@ -611,6 +636,12 @@ grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) > filebytes -= adlen; > ad++; > len -= sizeof (struct grub_udf_long_ad); > + > + if (GRUB_UDF_INVALID_STRUCT_PTR(ad, struct grub_udf_long_ad)) > + { > + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); > + return 0; > + } > } > } > > @@ -630,6 +661,7 @@ grub_udf_read_file (grub_fshelp_node_t node, > case GRUB_UDF_ICBTAG_FLAG_AD_IN_ICB: > { > char *ptr; > + char *end_ptr = (char *) node + get_fshelp_size (node->data); > > ptr = ((U16 (node->block.fe.tag.tag_ident) == GRUB_UDF_TAG_IDENT_FE) ? > ((char *) &node->block.fe.ext_attr[0] > @@ -637,6 +669,12 @@ grub_udf_read_file (grub_fshelp_node_t node, > ((char *) &node->block.efe.ext_attr[0] > + U32 (node->block.efe.ext_attr_length))); > > + if ((ptr + pos + len) > end_ptr) > + { > + grub_error (GRUB_ERR_BAD_FS, "corrupted UDF file system"); > + return 0; > + } > + > grub_memcpy (buf, ptr + pos, len); > > return len; > -- > 2.39.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] fs/udf: Fix out of bounds access 2023-06-07 19:26 ` Darren Kenny @ 2023-06-12 14:16 ` Daniel Kiper 0 siblings, 0 replies; 4+ messages in thread From: Daniel Kiper @ 2023-06-12 14:16 UTC (permalink / raw) To: Darren Kenny; +Cc: Lidong Chen, grub-devel On Wed, Jun 07, 2023 at 08:26:44PM +0100, Darren Kenny wrote: > Hi Li,, > > LGTM! > > Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Thank you for fixing these issues! Daniel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-12 14:17 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-07 1:31 [PATCH v2 0/1] fs/udf: Fix out of bounds access Lidong Chen 2023-06-07 1:31 ` [PATCH v2 1/1] " Lidong Chen 2023-06-07 19:26 ` Darren Kenny 2023-06-12 14:16 ` Daniel Kiper
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.