From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mOj0V-0003mH-AT for mharc-grub-devel@gnu.org; Fri, 10 Sep 2021 12:04:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:33008) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mOj0T-0003fJ-1y for grub-devel@gnu.org; Fri, 10 Sep 2021 12:04:01 -0400 Received: from mail-qk1-x72d.google.com ([2607:f8b0:4864:20::72d]:37480) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mOj0Q-0007FS-69 for grub-devel@gnu.org; Fri, 10 Sep 2021 12:04:00 -0400 Received: by mail-qk1-x72d.google.com with SMTP id w78so2460774qkb.4 for ; Fri, 10 Sep 2021 09:03:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=zJfrrhkwmOE38WLq02pxWlrqlWnqz7CcerqLpkOj7Ms=; b=vT7z/KkObxaoy+WcArP0Pys6TJ/El8MSZYLZ5JwfhlZUlOAXdHkp8Og20HYvs8g0Nz TnK3kOvAwg2R+FAfCgRcnTayijcyMQY60RJmfBiNaGxQBSKIrorxtjJtDtQCsXSU2IVC Bq4MA+up0D2PA8qXKsjyfT2S1aXoblbhcLn+8Px0sFKflZ9X8iNzQRYUxAGtRVAHSN5z B9nic1/dGAbPsTEryDXetUfKXBh67sLaye5q4l4ylwMgaYlkhCCidnqNolmQayrZRKdl xE6rbjS/bAybkzCQxanOnNnsk4GLi0zRK3/PBNYJUyjZbvEA0wDIrmc2caisfIjDEEiM kdng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=zJfrrhkwmOE38WLq02pxWlrqlWnqz7CcerqLpkOj7Ms=; b=zMtFcNC+mEva9RsjkGsQr8re10VsQeeSgH+qCAr/X26m6eevvQFEkgFr8MHCgAbLPm SDpMZaLbEKh6r6kO6WDIcEHaH1tgL3vebtiwMhLrWZ6OmnPRF59LtnHj11i+CMBFuuaS h2wiNqjpl1qBm4sQwhQesFob8x8aLRjRGSBRK+K6PcdUbn3H293RjRWBcfeOcIZrV+EN 2wbR9GLGESJfLsAvA2Czj7KNuR16bzEXiC1q9qawWOBEuFH7ZVCTxf669hzwK+eNPrj4 FWwFn0TfZcAmY4fXj3hvE+6Nbc2Zykpp3UOx7uQIOYtv/7bROXaqPv84pkK6z4obhiDR vz3A== X-Gm-Message-State: AOAM531z5ngrHpIgl/1rFAVbT46p7z/GFKtDbSf7T9kH+YiqtqDwvIdL tssTNZfuJi+JNEkNuWe8HMsxkPnLOUx3A+ZI X-Google-Smtp-Source: ABdhPJybsqo1QLWTK3W0ichbGHB7SZU4OHXoY6TUZltJZpOFUTeRfNZfqAQvaBsYlIrOBhn9oo0O/w== X-Received: by 2002:a05:620a:14ad:: with SMTP id x13mr8437428qkj.172.1631289834763; Fri, 10 Sep 2021 09:03:54 -0700 (PDT) Received: from ubuntu.. ([199.58.83.9]) by smtp.gmail.com with ESMTPSA id d6sm3847236qkn.119.2021.09.10.09.03.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Sep 2021 09:03:54 -0700 (PDT) From: Glenn Washburn To: grub-devel@gnu.org, Daniel Kiper Cc: Vladimir 'phcoder' Serbinenko , Peter Jones , Glenn Washburn Subject: [PATCH] udf: Fix regression which is preventing symlink access Date: Fri, 10 Sep 2021 16:03:23 +0000 Message-Id: <20210910160323.1247372-1-development@efficientek.com> X-Mailer: git-send-email 2.32.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2607:f8b0:4864:20::72d; envelope-from=development@efficientek.com; helo=mail-qk1-x72d.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Sep 2021 16:04:01 -0000 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 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 worst case upper- bound for the needed output buffer size is (sz-4)*1.5, where 4 is the size of a path component header and 1.5 is the maximum growth in bytes when converting from 2-byte unicode code-points to UTF-8 (from 2 bytes to 3). Signed-off-by: Glenn Washburn --- grub-core/fs/udf.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c index 2ac5c1d00..197e60d12 100644 --- a/grub-core/fs/udf.c +++ b/grub-core/fs/udf.c @@ -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 2-byte character string. Allocate 2*sz for the + * output buffer contained the string converted to UTF-8 because the + * resulting string can not be more than double the size (2-byte unicode + * 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)) -- 2.32.0