[pacman-dev] [PATCH 1/2] Enforce signature download size limit on -U <url> operations
We had a 16 KiB limit on database signatures, we should do the same here too to have a slight sanity check, even if we can't do so for the package itself yet. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/dload.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index efd469d..2928590 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -604,6 +604,9 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, const char *url) payload.force = 1; payload.errors_ok = (handle->siglevel & ALPM_SIG_PACKAGE_OPTIONAL); + /* set hard upper limit of 16KiB */ + payload.max_size = 16 * 1024; + ret = _alpm_download(&payload, cachedir, &sig_final_file); if(ret == -1 && !payload.errors_ok) { _alpm_log(handle, ALPM_LOG_WARNING, -- 1.7.8
This prevents user error in adding a file generated via `gpg --sign` rather than `--detach-sign`, for example. The same 16KiB limit is used we use in our pacman download code. The section is moved above the checksum generation to avoid presenting info messages to the user if the signature isn't valid. Addresses a shortcoming pointed out in FS#27453. Signed-off-by: Dan McGee <dan@archlinux.org> --- scripts/repo-add.sh.in | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 5e1d770..1cde913 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -245,7 +245,7 @@ db_write_entry() { local pkgfile="$1" local -a _groups _licenses _replaces _depends _conflicts _provides _optdepends local pkgname pkgver pkgdesc csize size url arch builddate packager \ - md5sum sha256sum pgpsig + md5sum sha256sum pgpsig pgpsigsize # read info from the zipped package local line var val @@ -284,6 +284,17 @@ db_write_entry() { fi fi + # compute base64'd PGP signature + if [[ -f "$pkgfile.sig" ]]; then + pgpsigsize=$(@SIZECMD@ "$pkgfile.sig") + if [[ $pgpsigsize > 16384 ]]; then + error "$(gettext "Invalid package signature file '%s'.")" "$pkgfile.sig" + return 1 + fi + msg2 "$(gettext "Adding package signature...")" + pgpsig=$(openssl base64 -in "$pkgfile.sig" | tr -d '\n') + fi + csize=$(@SIZECMD@ "$pkgfile") # compute checksums @@ -293,12 +304,6 @@ db_write_entry() { sha256sum="$(openssl dgst -sha256 "$pkgfile")" sha256sum="${sha256sum##* }" - # compute base64'd PGP signature - if [[ -f "$pkgfile.sig" ]]; then - msg2 "$(gettext "Adding package signature...")" - pgpsig=$(openssl base64 -in "$pkgfile.sig" | tr -d '\n') - fi - # remove an existing entry if it exists, ignore failures db_remove_entry "$pkgname" -- 1.7.8
On Mon, Dec 05, 2011 at 10:09:45AM -0600, Dan McGee wrote:
This prevents user error in adding a file generated via `gpg --sign` rather than `--detach-sign`, for example. The same 16KiB limit is used we use in our pacman download code.
The section is moved above the checksum generation to avoid presenting info messages to the user if the signature isn't valid.
Addresses a shortcoming pointed out in FS#27453.
Signed-off-by: Dan McGee <dan@archlinux.org> --- scripts/repo-add.sh.in | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 5e1d770..1cde913 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -245,7 +245,7 @@ db_write_entry() { local pkgfile="$1" local -a _groups _licenses _replaces _depends _conflicts _provides _optdepends local pkgname pkgver pkgdesc csize size url arch builddate packager \ - md5sum sha256sum pgpsig + md5sum sha256sum pgpsig pgpsigsize
# read info from the zipped package local line var val @@ -284,6 +284,17 @@ db_write_entry() { fi fi
+ # compute base64'd PGP signature + if [[ -f "$pkgfile.sig" ]]; then + pgpsigsize=$(@SIZECMD@ "$pkgfile.sig") + if [[ $pgpsigsize > 16384 ]]; then
This is a lexical comparison -- most sigs are going to fail this check. You meant to use an arithmetic context: (( pgpsigsize > 16384 )) This looks ifne otherwise.
+ error "$(gettext "Invalid package signature file '%s'.")" "$pkgfile.sig" + return 1 + fi + msg2 "$(gettext "Adding package signature...")" + pgpsig=$(openssl base64 -in "$pkgfile.sig" | tr -d '\n') + fi + csize=$(@SIZECMD@ "$pkgfile")
# compute checksums @@ -293,12 +304,6 @@ db_write_entry() { sha256sum="$(openssl dgst -sha256 "$pkgfile")" sha256sum="${sha256sum##* }"
- # compute base64'd PGP signature - if [[ -f "$pkgfile.sig" ]]; then - msg2 "$(gettext "Adding package signature...")" - pgpsig=$(openssl base64 -in "$pkgfile.sig" | tr -d '\n') - fi - # remove an existing entry if it exists, ignore failures db_remove_entry "$pkgname"
-- 1.7.8
On Mon, Dec 5, 2011 at 10:33 AM, Dave Reisner <d@falconindy.com> wrote:
On Mon, Dec 05, 2011 at 10:09:45AM -0600, Dan McGee wrote:
This prevents user error in adding a file generated via `gpg --sign` rather than `--detach-sign`, for example. The same 16KiB limit is used we use in our pacman download code.
The section is moved above the checksum generation to avoid presenting info messages to the user if the signature isn't valid.
Addresses a shortcoming pointed out in FS#27453.
Signed-off-by: Dan McGee <dan@archlinux.org> --- scripts/repo-add.sh.in | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 5e1d770..1cde913 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -245,7 +245,7 @@ db_write_entry() { local pkgfile="$1" local -a _groups _licenses _replaces _depends _conflicts _provides _optdepends local pkgname pkgver pkgdesc csize size url arch builddate packager \ - md5sum sha256sum pgpsig + md5sum sha256sum pgpsig pgpsigsize
# read info from the zipped package local line var val @@ -284,6 +284,17 @@ db_write_entry() { fi fi
+ # compute base64'd PGP signature + if [[ -f "$pkgfile.sig" ]]; then + pgpsigsize=$(@SIZECMD@ "$pkgfile.sig") + if [[ $pgpsigsize > 16384 ]]; then
This is a lexical comparison -- most sigs are going to fail this check. You meant to use an arithmetic context:
(( pgpsigsize > 16384 )) Bagh, yeah, we can do this, I'll update the patch. I thought [[ made '>' work rather than dropping back to '-gt'. Of course, my testing locally worked, perhaps by chance.
This looks ifne otherwise.
Thanks for the review.
+ error "$(gettext "Invalid package signature file '%s'.")" "$pkgfile.sig" + return 1 + fi + msg2 "$(gettext "Adding package signature...")" + pgpsig=$(openssl base64 -in "$pkgfile.sig" | tr -d '\n') + fi + csize=$(@SIZECMD@ "$pkgfile")
# compute checksums @@ -293,12 +304,6 @@ db_write_entry() { sha256sum="$(openssl dgst -sha256 "$pkgfile")" sha256sum="${sha256sum##* }"
- # compute base64'd PGP signature - if [[ -f "$pkgfile.sig" ]]; then - msg2 "$(gettext "Adding package signature...")" - pgpsig=$(openssl base64 -in "$pkgfile.sig" | tr -d '\n') - fi - # remove an existing entry if it exists, ignore failures db_remove_entry "$pkgname"
-- 1.7.8
participants (3)
-
Dan McGee
-
Dan McGee
-
Dave Reisner