Judge an upstream pin's host like any name, and harden the upstream unpack (review 35, hq issue 345)
The url was skipped by the installation check, so an installation's host or an address passed; it is judged whole now, the vendor's host declared in the artifact's names-on-purpose, and an address or a reserved name as the host is refused. Only the version's dotted-quad reading is skipped. The unpack caps its entries, drains xz before waiting for it, reads a pax global header and a nested within's parents, and a redirect off https is not followed.
This commit is contained in:
@@ -39,6 +39,22 @@ import (
|
||||
// unpacks no archive larger than this either (mesh-host internal/apply/archive.go, maxArchive).
|
||||
const maxUpstream = 512 << 20
|
||||
|
||||
// maxEntries is the most entries an upstream archive may hold: millions of empty ones would fill the build
|
||||
// seat's inodes and the node's. Betterbird's release holds 64.
|
||||
const maxEntries = 100_000
|
||||
|
||||
// noDowngrade refuses a redirect off https: the digest still guards the bytes, and a fetch in the clear says
|
||||
// to whoever is on the path what the build seat fetches.
|
||||
func noDowngrade(req *http.Request, via []*http.Request) error {
|
||||
if req.URL.Scheme != "https" {
|
||||
return fmt.Errorf("%s redirected to %s, which is not https", via[0].URL, req.URL)
|
||||
}
|
||||
if len(via) >= 10 {
|
||||
return errors.New("more than 10 redirects")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// upstreamClient fetches upstream files: a variable so a test serves its own.
|
||||
var upstreamClient = &http.Client{Timeout: 15 * time.Minute}
|
||||
|
||||
@@ -98,7 +114,9 @@ func upstreamFile(ctx context.Context, workspace string, a catalogue.Artifact,
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
response, err := upstreamClient.Do(request)
|
||||
client := *upstreamClient
|
||||
client.CheckRedirect = noDowngrade
|
||||
response, err := client.Do(request)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("cannot fetch %s: %w", a.URL, err)
|
||||
}
|
||||
@@ -213,6 +231,10 @@ func unpackUpstream(ctx context.Context, file, into, within string) error {
|
||||
return err
|
||||
}
|
||||
if wait != nil {
|
||||
// Whatever follows the tar's end is read before xz is waited for: xz blocks on a full pipe otherwise.
|
||||
if _, err := io.Copy(io.Discard, stream); err != nil {
|
||||
return err
|
||||
}
|
||||
w := wait
|
||||
wait = nil
|
||||
return w()
|
||||
@@ -228,7 +250,7 @@ func untar(stream io.Reader, into, within string) error {
|
||||
prefix = path.Clean(within) + "/"
|
||||
}
|
||||
var total int64
|
||||
files := 0
|
||||
files, entries := 0, 0
|
||||
for {
|
||||
header, err := reader.Next()
|
||||
if err == io.EOF {
|
||||
@@ -237,11 +259,20 @@ func untar(stream io.Reader, into, within string) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if entries++; entries > maxEntries {
|
||||
return fmt.Errorf("it holds more than %d entries", maxEntries)
|
||||
}
|
||||
if header.Typeflag == tar.TypeXGlobalHeader {
|
||||
// A pax global header (git archive writes one) names no file.
|
||||
continue
|
||||
}
|
||||
name := path.Clean(strings.TrimPrefix(header.Name, "./"))
|
||||
if path.IsAbs(name) || name == ".." || strings.HasPrefix(name, "../") {
|
||||
return fmt.Errorf("%s names a path outside the archive", header.Name)
|
||||
}
|
||||
if name == "." || name+"/" == prefix {
|
||||
if name == "." || name+"/" == prefix ||
|
||||
(header.Typeflag == tar.TypeDir && strings.HasPrefix(prefix, name+"/")) {
|
||||
// The archive's root, the directory taken, or one of its parents.
|
||||
continue
|
||||
}
|
||||
if prefix != "" {
|
||||
|
||||
Reference in New Issue
Block a user