Skip to content

Commit f5748c3

Browse files
committed
fix(taskfile): verify the pinned checksum on remote cache hits
A cache hit returned the cached Taskfile straight away, without ever comparing it to the checksum pinned on the include. The pin was only ever checked against freshly downloaded bytes, so a tampered cache entry bypassed it entirely. The same applies to the offline and cancelled download paths. A timestamp dated in the future also kept an entry valid past any expiry duration, since nothing bounded the value read back from disk.
1 parent 4c144fe commit f5748c3

1 file changed

Lines changed: 16 additions & 4 deletions

File tree

taskfile/reader.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -470,12 +470,24 @@ func (r *Reader) readNodeContent(ctx context.Context, node Node) ([]byte, error)
470470
return b, nil
471471
}
472472

473+
func verifyPinnedChecksum(node RemoteNode, b []byte) ([]byte, error) {
474+
checksum := checksum(b)
475+
if !node.Verify(checksum) {
476+
return nil, &errors.TaskfileDoesNotMatchChecksum{
477+
URI: node.Location(),
478+
ExpectedChecksum: node.Checksum(),
479+
ActualChecksum: checksum,
480+
}
481+
}
482+
return b, nil
483+
}
484+
473485
func (r *Reader) readRemoteNodeContent(ctx context.Context, node RemoteNode) ([]byte, error) {
474486
cache := NewCacheNode(node, r.tempDir)
475487
now := time.Now().UTC()
476488
timestamp := cache.ReadTimestamp()
477489
expiry := timestamp.Add(r.cacheExpiryDuration)
478-
cacheValid := now.Before(expiry)
490+
cacheValid := !timestamp.After(now) && now.Before(expiry)
479491
var cacheFound bool
480492

481493
r.debugf("checking cache for %q in %q\n", node.Location(), cache.Location())
@@ -498,7 +510,7 @@ func (r *Reader) readRemoteNodeContent(ctx context.Context, node RemoteNode) ([]
498510
// If we can't fetch a fresh copy, we should use the cache anyway
499511
if r.offline {
500512
r.debugf("in offline mode, using expired cache\n")
501-
return cachedBytes, nil
513+
return verifyPinnedChecksum(node, cachedBytes)
502514
}
503515

504516
// Some other error
@@ -510,7 +522,7 @@ func (r *Reader) readRemoteNodeContent(ctx context.Context, node RemoteNode) ([]
510522
r.debugf("cache found\n")
511523
// Not being forced to redownload, return cache
512524
if !r.download {
513-
return cachedBytes, nil
525+
return verifyPinnedChecksum(node, cachedBytes)
514526
}
515527
cacheFound = true
516528
}
@@ -526,7 +538,7 @@ func (r *Reader) readRemoteNodeContent(ctx context.Context, node RemoteNode) ([]
526538
} else {
527539
r.debugf("failed to fetch remote file: %s: using expired cache\n", ctx.Err().Error())
528540
}
529-
return cachedBytes, nil
541+
return verifyPinnedChecksum(node, cachedBytes)
530542
}
531543
return nil, err
532544
}

0 commit comments

Comments
 (0)