]> git.ipfire.org Git - thirdparty/grub.git/commitdiff
fs/udf: Fix regression which is preventing symlink access
authorGlenn Washburn <development@efficientek.com>
Fri, 17 Sep 2021 20:28:05 +0000 (20:28 +0000)
committerDaniel Kiper <daniel.kiper@oracle.com>
Mon, 20 Sep 2021 11:52:55 +0000 (13:52 +0200)
This code was broken by commit 3f05d693 (malloc: Use overflow checking
primitives where we do complex allocations), which added overflow
checking in many areas. The problem here is that the changes update the
local variable sz, which was already in use and which was not updated
before the change. So the code using sz was getting a different value of
than it would have previously for the same UDF image. This causes the
logic getting the destination of the symlink to not realize that its
gotten the full destination, but keeps trying to read past the end of
the destination. The bytes after the end are generally NULL padding
bytes, but that's not a valid component type (ECMA-167 14.16.1.1). So
grub_udf_read_symlink() branches to error logic, returning NULL, instead
of the symlink destination path.

The result of this bug is that the UDF filesystem tests were failing in
the symlink test with the grub-fstest error message:

  grub-fstest: error: cannot open `(loop0)/sym': invalid symlink.

This change stores the result of doubling sz in another local variable s,
so as not to modify sz. Also remove unnecessary grub_add(), which increased
the output by 1, presumably to account for a NULL byte. This isn't needed
because an output buffer of size twice sz is already guaranteed to be more
than enough to contain the path components converted to UTF-8. The value of
sz contains at least 4 bytes for the path component header (ECMA-167 14.16.1),
which means that 2 * 4 bytes are allocated but will not be used for UTF-8
characters, so the NULL byte is accounted for.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
grub-core/fs/udf.c

index 2ac5c1d0048fb3b0df331e7a9c5e0120d391ce37..4ff97021c28775220fc447ed783da331f8e72cd5 100644 (file)
@@ -1022,7 +1022,7 @@ grub_udf_iterate_dir (grub_fshelp_node_t dir,
 static char *
 grub_udf_read_symlink (grub_fshelp_node_t node)
 {
-  grub_size_t sz = U64 (node->block.fe.file_size);
+  grub_size_t s, sz = U64 (node->block.fe.file_size);
   grub_uint8_t *raw;
   const grub_uint8_t *ptr;
   char *out = NULL, *optr;
@@ -1035,11 +1035,19 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
   if (grub_udf_read_file (node, NULL, NULL, 0, sz, (char *) raw) < 0)
     goto fail_1;
 
-  if (grub_mul (sz, 2, &sz) ||
-      grub_add (sz, 1, &sz))
+  /*
+   * Local sz is the size of the symlink file data, which contains a sequence
+   * of path components (ECMA-167 14.16.1) representing the link destination.
+   * This size is an upper-bound on the number of bytes of a contained and
+   * potentially compressed UTF-16 character string. Allocate 2*sz for the
+   * output buffer containing the string converted to UTF-8 because the
+   * resulting string can not be more than double the size (2-byte unicode
+   * code points will be converted to a maximum of 3 bytes in UTF-8).
+   */
+  if (grub_mul (sz, 2, &s))
     goto fail_0;
 
-  out = grub_malloc (sz);
+  out = grub_malloc (s);
   if (!out)
     {
  fail_0:
@@ -1051,7 +1059,6 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
 
   for (ptr = raw; ptr < raw + sz; )
     {
-      grub_size_t s;
       if ((grub_size_t) (ptr - raw + 4) > sz)
        goto fail_1;
       if (!(ptr[2] == 0 && ptr[3] == 0))