GNU bug report logs - #19479
Package manager vulnerable

Previous Next

Full log

View this message in rfc822 format

From: Noam Postavsky <npostavs <at> gmail.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 19479 <at> debbugs.gnu.org
Subject: bug#19479: Package manager vulnerable
Date: Tue, 05 May 2020 20:55:53 -0400

Stefan Kangas <stefan <at> marxist.se> writes:

> Subject: [PATCH] Support package checksum verification
>
> This is the first step towards protecting users of package.el against
> metadata replay attacks.

> +(define-error 'bad-checksum "Failed to verify checksum")

Would it be useful to have bad-signature and this one share a parent?
(by the way, I kind of wonder why it's not called
package-bad-signature).

> +  (cl-flet*
> +      ((supported-hashes
> +        (lambda ()

Is this a function (rather than a variable) just so it can be in the
same cl-flet* as do-check?

> +          (or (seq-filter (lambda (h) (memql (car h) (secure-hash-algorithms)))

The list returned by secure-hash-algorithms includes sha1 and md5.  This
is a problem if we're going to rely on signing them.  We should at least
plan to have some way of filtering out some functions.

> +                   (a (cdr hash))
> +                   (b (secure-hash algorithm (current-buffer))))

> +  (when-let ((a (package-desc-size pkg-desc))
> +             (b (string-bytes (buffer-string))))

I risk descending into trivial nitpicking, but I think 'a' and 'b' are
bit too generic.  Something like 'expected' and 'actual' would make it
harder to mix them up.

> +(defmacro run-verify-checksums-test (verify-checksums checksums)
> +  "Run a test for `package-verify-checksums'."

> +(ert-deftest package-test--verify-package-checksums-nil/ignore-invalid ()

I think run-verify-checksums-test should be prefixed with package-test
(whereas the individual test names could be prefixed with just package).

Previous Next

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