mirror of
https://github.com/rclone/rclone.git
synced 2026-10-28 06:18:39 -04:00
selfupdate: fix hash check trusting unsigned data in the signed SHA256SUMS GHSA-w33h-3qgq-8mv8 CVE-PENDING
rclone selfupdate checked the PGP signature on the clearsigned block of SHA256SUMS but then looked up the archive hash in the whole downloaded file. Unsigned data can appear in that file without invalidating the signature. A hash line placed before or after the signed text was accepted, allowing whoever controlled the download to install an arbitrary unsigned archive. The hash is now only looked up in the signed text. Affects stable (non --beta) selfupdate since v1.55.0. --beta updates are not signature checked.
This commit is contained in:
1 parent
bc1050bc8a
commit
672bbf75ec
2 files changed
+21
-1
No files matched your search
@@ -86,7 +86,8 @@ func verifyHashsumDownloaded(ctx context.Context, sumsBuf []byte, archive string
|
||||
return fmt.Errorf("invalid hashsum signature: %w", err)
|
||||
}
|
||||
|
||||
wantHash, err := findFileHash(sumsBuf, archive)
|
||||
// Only read hashes from the the signed text
|
||||
wantHash, err := findFileHash(block.Bytes, archive)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"context"
|
||||
"encoding/hex"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -39,4 +40,22 @@ func TestVerify(t *testing.T) {
|
||||
err = verifyHashsumDownloaded(ctx, sumsBuf, "archive.zipX", hash)
|
||||
assert.ErrorContains(t, err, "unable to find hash")
|
||||
})
|
||||
t.Run("UnsignedPrefix", func(t *testing.T) {
|
||||
hash[0] ^= 1 // forge an unsigned hash line before the signed block
|
||||
prefixed := append([]byte(hex.EncodeToString(hash)+" archive.zip\n"), sumsBuf...)
|
||||
err = verifyHashsumDownloaded(ctx, prefixed, "archive.zip", hash)
|
||||
assert.ErrorContains(t, err, "archive hash mismatch")
|
||||
hash[0] ^= 1 // undo the change
|
||||
})
|
||||
t.Run("UnsignedInArmor", func(t *testing.T) {
|
||||
// Dash escaping the signed line doesn't change the signed text but hides
|
||||
// it from a raw scan, and lines after the armor checksum are ignored.
|
||||
badHash := hex.EncodeToString([]byte{0x01})
|
||||
armored := strings.Replace(string(sumsBuf), "b20b", "- b20b", 1)
|
||||
armored = strings.Replace(armored, "=1GTr\n", "=1GTr\n"+badHash+" archive.zip\n", 1)
|
||||
err = verifyHashsumDownloaded(ctx, []byte(armored), "archive.zip", []byte{0x01})
|
||||
assert.ErrorContains(t, err, "archive hash mismatch")
|
||||
err = verifyHashsumDownloaded(ctx, []byte(armored), "archive.zip", hash)
|
||||
require.NoError(t, err)
|
||||
})
|
||||
}
|
||||
Reference in new issue
Block a user