From: Paul Eggert <eggert@cs.ucla.edu>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>,
Florian Weimer <fweimer@redhat.com>,
bug-gnulib@gnu.org
Subject: Re: Undefined behavior in lib/canonicalize-lgpl.c
Date: Thu, 10 Sep 2020 14:52:44 -0700 [thread overview]
Message-ID: <24e3db52-d372-cdf5-d442-2c8aee0cf804@cs.ucla.edu> (raw)
In-Reply-To: <175640c6-80c9-a6a4-2281-421f4a6bc519@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 374 bytes --]
On 9/10/20 6:34 AM, Adhemerval Zanella wrote:
> I think the lib/canonicalize.c is also affected:
Thanks for the heads-ups. I installed the attached patch to Gnulib, which fixes
a related bug I noticed while in the neighborhood (I don't think glibc has this
bug).
This patch uses the expression (rpath_limit - dest <= end - start) which I found
a little easier to grok.
[-- Attachment #2: 0001-canonicalize-fix-pointer-indexing-bugs.patch --]
[-- Type: text/x-patch, Size: 3266 bytes --]
From d468be5b5950bc5f2e0a5a4fbaeb0ea6a88c4c9f Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 10 Sep 2020 14:25:51 -0700
Subject: [PATCH] canonicalize: fix pointer indexing bugs
Problem reported by Florian Weimer in:
https://lists.gnu.org/r/bug-gnulib/2020-09/msg00025.html
* lib/canonicalize-lgpl.c (__realpath):
* lib/canonicalize.c (canonicalize_filename_mode):
Do not generate a pointer past the end of the array.
* lib/canonicalize.c (canonicalize_filename_mode):
Do not use a pointer after passing it to realloc.
---
ChangeLog | 11 +++++++++++
lib/canonicalize-lgpl.c | 2 +-
lib/canonicalize.c | 19 ++++++++-----------
3 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 8b6f62bb1..bf39cc512 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2020-09-10 Paul Eggert <eggert@cs.ucla.edu>
+
+ canonicalize: fix pointer indexing bugs
+ Problem reported by Florian Weimer in:
+ https://lists.gnu.org/r/bug-gnulib/2020-09/msg00025.html
+ * lib/canonicalize-lgpl.c (__realpath):
+ * lib/canonicalize.c (canonicalize_filename_mode):
+ Do not generate a pointer past the end of the array.
+ * lib/canonicalize.c (canonicalize_filename_mode):
+ Do not use a pointer after passing it to realloc.
+
2020-09-09 Paul Eggert <eggert@cs.ucla.edu>
tempname: help merge with glibc
diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c
index 0b89d2a18..cc42662db 100644
--- a/lib/canonicalize-lgpl.c
+++ b/lib/canonicalize-lgpl.c
@@ -234,7 +234,7 @@ __realpath (const char *name, char *resolved)
if (!ISSLASH (dest[-1]))
*dest++ = '/';
- if (dest + (end - start) >= rpath_limit)
+ if (rpath_limit - dest <= end - start)
{
ptrdiff_t dest_offset = dest - rpath;
char *new_rpath;
diff --git a/lib/canonicalize.c b/lib/canonicalize.c
index 8bb325414..aa0c3bd28 100644
--- a/lib/canonicalize.c
+++ b/lib/canonicalize.c
@@ -138,18 +138,15 @@ canonicalize_filename_mode (const char *name, canonicalize_mode_t can_mode)
rname = xgetcwd ();
if (!rname)
return NULL;
- dest = strchr (rname, '\0');
- if (dest - rname < PATH_MAX)
+ size_t rnamelen = strlen (rname);
+ size_t rnamesize = rnamelen; /* Lower bound on size; good enough. */
+ if (rnamesize < PATH_MAX)
{
- char *p = xrealloc (rname, PATH_MAX);
- dest = p + (dest - rname);
- rname = p;
- rname_limit = rname + PATH_MAX;
- }
- else
- {
- rname_limit = dest;
+ rnamesize = PATH_MAX;
+ rname = xrealloc (rname, rnamesize);
}
+ dest = rname + rnamelen;
+ rname_limit = rname + rnamesize;
start = name;
prefix_len = FILE_SYSTEM_PREFIX_LEN (rname);
}
@@ -204,7 +201,7 @@ canonicalize_filename_mode (const char *name, canonicalize_mode_t can_mode)
if (!ISSLASH (dest[-1]))
*dest++ = '/';
- if (dest + (end - start) >= rname_limit)
+ if (rname_limit - dest <= end - start)
{
ptrdiff_t dest_offset = dest - rname;
size_t new_size = rname_limit - rname;
--
2.17.1
prev parent reply other threads:[~2020-09-10 21:52 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-10 8:32 Undefined behavior in lib/canonicalize-lgpl.c Florian Weimer
2020-09-10 13:34 ` Adhemerval Zanella
2020-09-10 21:52 ` Paul Eggert [this message]
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
List information: https://lists.gnu.org/mailman/listinfo/bug-gnulib
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=24e3db52-d372-cdf5-d442-2c8aee0cf804@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=adhemerval.zanella@linaro.org \
--cc=bug-gnulib@gnu.org \
--cc=fweimer@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).