From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Federico Tedin Newsgroups: gmane.emacs.bugs 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 Message-ID: <8735q4zcdh.fsf@gmail.com> References: <83o8ary5kl.fsf@gnu.org> <87pmtbj81v.fsf@gmail.com> <8335q7c655.fsf@gnu.org> <87pmta6buq.fsf@gmail.com> <837dfgaerv.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="26670"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) Cc: phst@google.com, 49723@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Sep 16 18:59:11 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mQuj8-0006jL-Q3 for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 16 Sep 2021 18:59:10 +0200 Original-Received: from localhost ([::1]:54880 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQuj7-0007Di-6i for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 16 Sep 2021 12:59:09 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:34692) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQuj0-0007DL-8d for bug-gnu-emacs@gnu.org; Thu, 16 Sep 2021 12:59:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:44886) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mQuj0-0001Nm-1T for bug-gnu-emacs@gnu.org; Thu, 16 Sep 2021 12:59:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mQuiz-0008Mr-Lu for bug-gnu-emacs@gnu.org; Thu, 16 Sep 2021 12:59:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Federico Tedin Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 16 Sep 2021 16:59:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 49723 X-GNU-PR-Package: emacs Original-Received: via spool by 49723-submit@debbugs.gnu.org id=B49723.163181149932104 (code B ref 49723); Thu, 16 Sep 2021 16:59:01 +0000 Original-Received: (at 49723) by debbugs.gnu.org; 16 Sep 2021 16:58:19 +0000 Original-Received: from localhost ([127.0.0.1]:56432 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mQuiI-0008Lk-Kd for submit@debbugs.gnu.org; Thu, 16 Sep 2021 12:58:19 -0400 Original-Received: from mail-wm1-f50.google.com ([209.85.128.50]:52113) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mQuiG-0008LU-BQ for 49723@debbugs.gnu.org; Thu, 16 Sep 2021 12:58:17 -0400 Original-Received: by mail-wm1-f50.google.com with SMTP id y132so5312934wmc.1 for <49723@debbugs.gnu.org>; Thu, 16 Sep 2021 09:58:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=JoW9pCBZPXRzti24/EsF5AhAAANPaz6b9qpNjI7HTM4=; b=nJgGdqJUDm730OgHea2jZHj5JrJriLU5oFducKUOA+Pxtc1vraoq7+cQ4q82z0efPI Doh3QGbIscJbo7XejTqKec8kwH3gF5GTbH2rorIkPcxO1TM+/8E8DB0SV0na+qBv+C7u bQSEA2phj7Ou+E2G6U1KagoF2BjkPRj98xoelmOYvt73vePGmOSstavdMa2q2/1n8gfY DyZNi/RA/R1VarcVZyGrX6xKwGqqf+Nkpoil2BoumrxvDWHLRKGCkayYMqvknMb3LsHM yX1loz1iqTcL+OQQCDplojAD8QoKi0chXaq+CHrUwNXYNZ2lqr/llA6978txn1kZq6u7 Bmuw== 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:references:date:in-reply-to :message-id:user-agent:mime-version; bh=JoW9pCBZPXRzti24/EsF5AhAAANPaz6b9qpNjI7HTM4=; b=H6TxC+BPQlQmHg+etpuFM8hNTRup+AZk9odtqrI4P46qkDWx+R3BJJPFbS99IMrgN4 sX3oQc4mBHiFA1Lm+tEd1DHBpcpjtdIUoInbe1OcELyROYwby45wFHGsTfqUwv6Kklxh rQmWmR3B4xoHXKRHioHIj8Ncksjg9LUihh/4+AaqTq+atWHxaM32jps1UrERyO4JC3V3 YrxKegDDfNIYGdkZ6r1VKtGMWVvQ+h0RKud3X6HKWCmJOWHlTko+OeJhi9uAW7Y85m+X 0Zd9ejlMNtIwDATb23cFHyVEmj+ykXxJ0+5Pxfdv6p8IsqADKj8UJvnLvZfjrQcAmFBg Cmdg== X-Gm-Message-State: AOAM532wgP2YQNgoJiLT/PVbOKtFg0cQeL76bpwrE4JTVveI0PCCqerr TzMQz/t52fS4U1XHS86Dv/ynkO5O9XM= X-Google-Smtp-Source: ABdhPJw3432C06Qt15fkMDAsxF6UCawWM+5lyEa4/0zswsjLcrhWBA6PAFln6cvqTnVIn/QEpjMiHQ== X-Received: by 2002:a7b:c052:: with SMTP id u18mr10925904wmc.105.1631811490204; Thu, 16 Sep 2021 09:58:10 -0700 (PDT) Original-Received: from gehirn (ip5b4202e5.dynamic.kabel-deutschland.de. [91.66.2.229]) by smtp.gmail.com with ESMTPSA id i9sm8288361wmi.44.2021.09.16.09.58.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Sep 2021 09:58:09 -0700 (PDT) In-Reply-To: <837dfgaerv.fsf@gnu.org> (Eli Zaretskii's message of "Thu, 16 Sep 2021 15:25:24 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:214485 Archived-At: --=-=-= Content-Type: text/plain Eli Zaretskii 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. --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=expand-file-name.patch Content-Description: patch >From 6faba0f1107290d44a83d396a61747dea67f0f39 Mon Sep 17 00:00:00 2001 From: Federico Tedin 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 --=-=-=--