From: Federico Tedin <federicotedin@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: phst@google.com, 49723@debbugs.gnu.org
Subject: bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable
Date: Thu, 16 Sep 2021 18:58:02 +0200 [thread overview]
Message-ID: <8735q4zcdh.fsf@gmail.com> (raw)
In-Reply-To: <837dfgaerv.fsf@gnu.org> (Eli Zaretskii's message of "Thu, 16 Sep 2021 15:25:24 +0300")
[-- Attachment #1: Type: text/plain, Size: 433 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
>> +** 'expand-file-name' now checks for null bytes in filenames.
>> +The function will now check for null bytes in both NAME and
>> +DEFAULT-DIRECTORY arguments, as well as in the 'default-directory'
>> +buffer-local variable, assuming its value is used.
>
> This should say that if null bytes are found, expand-file-name will
> signal an error.
Makes sense! I'm attaching a revised version.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 4320 bytes --]
From 6faba0f1107290d44a83d396a61747dea67f0f39 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Wed, 15 Sep 2021 00:15:16 +0200
Subject: [PATCH] Add null byte checks to filenames in expand-file-name
(bug#49723)
* src/fileio.c (expand-file-name): Check for null bytes for both NAME
and DEFAULT-DIRECTORY arguments. Also check for null bytes in
buffer-local default-directory, assuming it is used.
* src/coding.c (encode_file_name): Use CHECK_STRING_NULL_BYTES.
* src/lisp.h (CHECK_STRING_NULL_BYTES): Add function for checking for
null bytes in Lisp strings.
* test/src/fileio-tests.el (fileio-test--expand-file-name-null-bytes):
Add test for new changes to expand-file-name.
* etc/NEWS: Announce changes.
---
etc/NEWS | 7 +++++++
src/coding.c | 3 +--
src/fileio.c | 6 +++++-
src/lisp.h | 7 +++++++
test/src/fileio-tests.el | 9 +++++++++
5 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index 17c42ce104..3484ef3f3f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -281,6 +281,13 @@ personalize the uniquified buffer name.
---
** 'remove-hook' is now an interactive command.
+** 'expand-file-name' now checks for null bytes in filenames.
+The function will now check for null bytes in both NAME and
+DEFAULT-DIRECTORY arguments, as well as in the 'default-directory'
+buffer-local variable, assuming its value is used. If null bytes are
+found, 'expand-file-name' will signal an error.
+
+---
** Frames
+++
diff --git a/src/coding.c b/src/coding.c
index d027c7d539..7030a53869 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -10430,8 +10430,7 @@ encode_file_name (Lisp_Object fname)
cause subtle bugs because the system would silently use a
different filename than expected. Perform this check after
encoding to not miss NUL bytes introduced through encoding. */
- CHECK_TYPE (memchr (SSDATA (encoded), '\0', SBYTES (encoded)) == NULL,
- Qfilenamep, fname);
+ CHECK_STRING_NULL_BYTES (encoded);
return encoded;
}
diff --git a/src/fileio.c b/src/fileio.c
index 0db8ed793b..3c13d3fe41 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -945,6 +945,7 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
USE_SAFE_ALLOCA;
CHECK_STRING (name);
+ CHECK_STRING_NULL_BYTES (name);
/* If the file name has special constructs in it,
call the corresponding file name handler. */
@@ -993,7 +994,10 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
if (STRINGP (dir))
{
if (file_name_absolute_no_tilde_p (dir))
- default_directory = dir;
+ {
+ CHECK_STRING_NULL_BYTES (dir);
+ default_directory = dir;
+ }
else
{
Lisp_Object absdir
diff --git a/src/lisp.h b/src/lisp.h
index 7bfc69b647..9716b34bae 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -1615,6 +1615,13 @@ STRING_SET_CHARS (Lisp_Object string, ptrdiff_t newsize)
XSTRING (string)->u.s.size = newsize;
}
+INLINE void
+CHECK_STRING_NULL_BYTES (Lisp_Object string)
+{
+ CHECK_TYPE (memchr (SSDATA (string), '\0', SBYTES (string)) == NULL,
+ Qfilenamep, string);
+}
+
/* A regular vector is just a header plus an array of Lisp_Objects. */
struct Lisp_Vector
diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el
index f4d123b426..438ebebb76 100644
--- a/test/src/fileio-tests.el
+++ b/test/src/fileio-tests.el
@@ -136,6 +136,15 @@ fileio-tests--relative-default-directory
(should (and (file-name-absolute-p name)
(not (eq (aref name 0) ?~))))))
+(ert-deftest fileio-test--expand-file-name-null-bytes ()
+ "Test that expand-file-name checks for null bytes in filenames."
+ (should-error (expand-file-name (concat "file" (char-to-string ?\0) ".txt"))
+ :type 'wrong-type-argument)
+ (should-error (expand-file-name "file.txt" (concat "dir" (char-to-string ?\0)))
+ :type 'wrong-type-argument)
+ (let ((default-directory (concat "dir" (char-to-string ?\0))))
+ (should-error (expand-file-name "file.txt") :type 'wrong-type-argument)))
+
(ert-deftest fileio-tests--file-name-absolute-p ()
"Test file-name-absolute-p."
(dolist (suffix '("" "/" "//" "/foo" "/foo/" "/foo//" "/foo/bar"))
--
2.25.1
next prev parent reply other threads:[~2021-09-16 16:58 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-24 17:39 bug#49723: 28.0.50; Test in coding.c for NUL bytes in filenames is not reliable Eli Zaretskii
2021-09-14 19:01 ` Federico Tedin
2021-09-14 19:24 ` Eli Zaretskii
2021-09-14 22:17 ` Federico Tedin
2021-09-16 12:25 ` Eli Zaretskii
2021-09-16 16:58 ` Federico Tedin [this message]
2021-09-16 18:12 ` Michael Albinus
2021-09-16 18:30 ` Eli Zaretskii
2021-09-16 18:44 ` Michael Albinus
2021-09-16 19:07 ` Eli Zaretskii
2021-09-16 18:38 ` Federico Tedin
2021-09-16 18:51 ` Michael Albinus
2021-09-16 19:13 ` Federico Tedin
2021-09-20 11:59 ` Michael Albinus
2021-09-17 7:49 ` Eli Zaretskii
2021-09-17 19:00 ` Federico Tedin
2021-09-18 6:51 ` Eli Zaretskii
2021-09-18 17:57 ` Federico Tedin
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://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8735q4zcdh.fsf@gmail.com \
--to=federicotedin@gmail.com \
--cc=49723@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=phst@google.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.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
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).