GNU bug report logs - #22311
25.1.50; package.el misused (read-from-string) will potentially cause "elpa/archives/xxx/archive-contents" file malformed

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 22311 in the body.
You can then email your comments to 22311 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tao Fang <fangtao0901 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1.50; package.el misused (read-from-string) will potentially cause
 "elpa/archives/xxx/archive-contents" file malformed
Date: Tue, 05 Jan 2016 23:33:45 +0800

Hi, all
  There is a misused function read-from-string in package.el L1485:

  1472	(defun package--download-one-archive (archive file &optional async)
  1473	  "Retrieve an archive file FILE from ARCHIVE, and cache it.
  1474	ARCHIVE should be a cons cell of the form (NAME . LOCATION),
  1475	similar to an entry in `package-alist'.  Save the cached copy to
  1476	\"archives/NAME/FILE\" in `package-user-dir'."
  1477	  (package--with-response-buffer (cdr archive) :file file
  1478	    :async async
  1479	    :error-form (package--update-downloads-in-progress archive)
  1480	    (let* ((location (cdr archive))
  1481	           (name (car archive))
  1482	           (content (buffer-string))
  1483	           (dir (expand-file-name (format "archives/%s" name) package-user-dir))
  1484	           (local-file (expand-file-name file dir)))
  1485	      (when (listp (read-from-string content))
  1486	        (make-directory dir t)
  1487	        (if (or (not package-check-signature)

listp checks return value of (read-from-string content) to make sure we
get file content with correct format, but as its doc says:
"
(read-from-string STRING &optional START END)

Read one Lisp expression which is represented as text by STRING.
Returns a cons: (OBJECT-READ . FINAL-STRING-INDEX).
"
(listp (read-from-string content)) will always return t, if archive-contents file download
finished with malformed content (e.g. error message return from proxy
server), it will be parsed and saved by mistake.

Simply replace (read-from-string) with (read) would resolve this, I think.

Message #8 received at 22311 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Tao Fang <fangtao0901 <at> gmail.com>
Cc: 22311 <at> debbugs.gnu.org
Subject: Re: bug#22311: 25.1.50;
 package.el misused (read-from-string) will potentially cause
 "elpa/archives/xxx/archive-contents" file malformed
Date: Sat, 16 Jun 2018 19:07:39 -0400

[Message part 1 (text/plain, inline)]

tags 22311 + patch
quit

Tao Fang <fangtao0901 <at> gmail.com> writes:

>   There is a misused function read-from-string in package.el L1485:
>
>   1472	(defun package--download-one-archive (archive file &optional async)

>   1485	      (when (listp (read-from-string content))

> (listp (read-from-string content)) will always return t, if archive-contents file download
> finished with malformed content (e.g. error message return from proxy
> server), it will be parsed and saved by mistake.
>
> Simply replace (read-from-string) with (read) would resolve this, I think.

Right, seems it's a regression in 25.1.  So I think the patch below
should go to emacs-26.

[0001-Detect-a-non-list-package-archive-content-properly-B.patch (text/x-diff, inline)]

From 1ef28a6ba81120c13135e28b32c8ae6e20c4a219 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sat, 16 Jun 2018 18:59:43 -0400
Subject: [PATCH] Detect a non-list package archive content properly
 (Bug#22311)

* lisp/emacs-lisp/package.el (package--download-one-archive): Use
`read' instead of `read-from-string'; the latter always returns a
cons, so the `listp' check on its return value doesn't make sense.  It
was changed from `read' to `read-from-string' in 2015-04-01 "*
emacs-lisp/package.el: Implement asynchronous refreshing", but that
change was not needed because `read' works fine on strings as well as
buffers.
---
 lisp/emacs-lisp/package.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index c56502236e..576a9bc7e7 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1532,7 +1532,7 @@ package--download-one-archive
            (content (buffer-string))
            (dir (expand-file-name (format "archives/%s" name) package-user-dir))
            (local-file (expand-file-name file dir)))
-      (when (listp (read-from-string content))
+      (when (listp (read content))
         (make-directory dir t)
         (if (or (not package-check-signature)
                 (member name package-unsigned-archives))
-- 
2.11.0

Message #13 received at 22311 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Tao Fang <fangtao0901 <at> gmail.com>
Cc: 22311 <at> debbugs.gnu.org
Subject: Re: bug#22311: 25.1.50;
 package.el misused (read-from-string) will potentially cause
 "elpa/archives/xxx/archive-contents" file malformed
Date: Tue, 26 Jun 2018 19:57:33 -0400

tags 22311 fixed
close 22311 26.2
quit

Noam Postavsky <npostavs <at> gmail.com> writes:

>> Simply replace (read-from-string) with (read) would resolve this, I think.
>
> Right, seems it's a regression in 25.1.  So I think the patch below
> should go to emacs-26.

Pushed.

[1: 6f6d525683]: 2018-06-26 19:56:04 -0400
  Detect a non-list package archive content properly (Bug#22311)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=6f6d525683d5731d55fcd801a66b078bd6ba8369

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.