Call .Validate() before digest.Digest.String() if necessary

... to prevent unexpected behavior on invalid values.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

docker/docker_client.go

@@ -952,6 +952,8 @@ func (c *dockerClient) detectProperties(ctx context.Context) error {
 	return c.detectPropertiesError
 }
 
+// fetchManifest fetches a manifest for (the repo of ref) + tagOrDigest.
+// The caller is responsible for ensuring tagOrDigest uses the expected format.
 func (c *dockerClient) fetchManifest(ctx context.Context, ref dockerReference, tagOrDigest string) ([]byte, string, error) {
 	path := fmt.Sprintf(manifestPath, reference.Path(ref.ref), tagOrDigest)
 	headers := map[string][]string{
@@ -1035,6 +1037,9 @@ func (c *dockerClient) getBlob(ctx context.Context, ref dockerReference, info ty
 		}
 	}
 
+	if err := info.Digest.Validate(); err != nil { // Make sure info.Digest.String() does not contain any unexpected characters
+		return nil, 0, err
+	}
 	path := fmt.Sprintf(blobsPath, reference.Path(ref.ref), info.Digest.String())
 	logrus.Debugf("Downloading %s", path)
 	res, err := c.makeRequest(ctx, http.MethodGet, path, nil, nil, v2Auth, nil)
@@ -1098,7 +1103,10 @@ func isManifestUnknownError(err error) bool {
 // digest in ref.
 // It returns (nil, nil) if the manifest does not exist.
 func (c *dockerClient) getSigstoreAttachmentManifest(ctx context.Context, ref dockerReference, digest digest.Digest) (*manifest.OCI1, error) {
-	tag := sigstoreAttachmentTag(digest)
+	tag, err := sigstoreAttachmentTag(digest)
+	if err != nil {
+		return nil, err
+	}
 	sigstoreRef, err := reference.WithTag(reference.TrimNamed(ref.ref), tag)
 	if err != nil {
 		return nil, err
@@ -1131,6 +1139,9 @@ func (c *dockerClient) getSigstoreAttachmentManifest(ctx context.Context, ref do
 // getExtensionsSignatures returns signatures from the X-Registry-Supports-Signatures API extension,
 // using the original data structures.
 func (c *dockerClient) getExtensionsSignatures(ctx context.Context, ref dockerReference, manifestDigest digest.Digest) (*extensionSignatureList, error) {
+	if err := manifestDigest.Validate(); err != nil { // Make sure manifestDigest.String() does not contain any unexpected characters
+		return nil, err
+	}
 	path := fmt.Sprintf(extensionsSignaturePath, reference.Path(ref.ref), manifestDigest)
 	res, err := c.makeRequest(ctx, http.MethodGet, path, nil, nil, v2Auth, nil)
 	if err != nil {
@@ -1154,8 +1165,11 @@ func (c *dockerClient) getExtensionsSignatures(ctx context.Context, ref dockerRe
 }
 
 // sigstoreAttachmentTag returns a sigstore attachment tag for the specified digest.
-func sigstoreAttachmentTag(d digest.Digest) string {
-	return strings.Replace(d.String(), ":", "-", 1) + ".sig"
+func sigstoreAttachmentTag(d digest.Digest) (string, error) {
+	if err := d.Validate(); err != nil { // Make sure d.String() doesn’t contain any unexpected characters
+		return "", err
+	}
+	return strings.Replace(d.String(), ":", "-", 1) + ".sig", nil
 }
 
 // Close removes resources associated with an initialized dockerClient, if any.

docker/docker_image_dest.go

@@ -230,6 +230,9 @@ func (d *dockerImageDestination) PutBlobWithOptions(ctx context.Context, stream
 // If the destination does not contain the blob, or it is unknown, blobExists ordinarily returns (false, -1, nil);
 // it returns a non-nil error only on an unexpected failure.
 func (d *dockerImageDestination) blobExists(ctx context.Context, repo reference.Named, digest digest.Digest, extraScope *authScope) (bool, int64, error) {
+	if err := digest.Validate(); err != nil { // Make sure digest.String() does not contain any unexpected characters
+		return false, -1, err
+	}
 	checkPath := fmt.Sprintf(blobsPath, reference.Path(repo), digest.String())
 	logrus.Debugf("Checking %s", checkPath)
 	res, err := d.c.makeRequest(ctx, http.MethodHead, checkPath, nil, nil, v2Auth, extraScope)
@@ -469,6 +472,7 @@ func (d *dockerImageDestination) PutManifest(ctx context.Context, m []byte, inst
 		// particular instance.
 		refTail = instanceDigest.String()
 		// Double-check that the manifest we've been given matches the digest we've been given.
+		// This also validates the format of instanceDigest.
 		matches, err := manifest.MatchesDigest(m, *instanceDigest)
 		if err != nil {
 			return fmt.Errorf("digesting manifest in PutManifest: %w", err)
@@ -783,8 +787,12 @@ func (d *dockerImageDestination) putSignaturesToSigstoreAttachments(ctx context.
 	if err != nil {
 		return err
 	}
+	attachmentTag, err := sigstoreAttachmentTag(manifestDigest)
+	if err != nil {
+		return err
+	}
 	logrus.Debugf("Uploading sigstore attachment manifest")
-	return d.uploadManifest(ctx, manifestBlob, sigstoreAttachmentTag(manifestDigest))
+	return d.uploadManifest(ctx, manifestBlob, attachmentTag)
 }
 
 func layerMatchesSigstoreSignature(layer imgspecv1.Descriptor, mimeType string,
@@ -900,6 +908,7 @@ func (d *dockerImageDestination) putSignaturesToAPIExtension(ctx context.Context
 			return err
 		}
 
+		// manifestDigest is known to be valid because it was not rejected by getExtensionsSignatures above.
 		path := fmt.Sprintf(extensionsSignaturePath, reference.Path(d.ref.ref), manifestDigest.String())
 		res, err := d.c.makeRequest(ctx, http.MethodPut, path, nil, bytes.NewReader(body), v2Auth, nil)
 		if err != nil {

docker/docker_image_src.go

@@ -194,6 +194,9 @@ func simplifyContentType(contentType string) string {
 // this never happens if the primary manifest is not a manifest list (e.g. if the source never returns manifest lists).
 func (s *dockerImageSource) GetManifest(ctx context.Context, instanceDigest *digest.Digest) ([]byte, string, error) {
 	if instanceDigest != nil {
+		if err := instanceDigest.Validate(); err != nil { // Make sure instanceDigest.String() does not contain any unexpected characters
+			return nil, "", err
+		}
 		return s.fetchManifest(ctx, instanceDigest.String())
 	}
 	err := s.ensureManifestIsLoaded(ctx)
@@ -203,6 +206,8 @@ func (s *dockerImageSource) GetManifest(ctx context.Context, instanceDigest *dig
 	return s.cachedManifest, s.cachedManifestMIMEType, nil
 }
 
+// fetchManifest fetches a manifest for tagOrDigest.
+// The caller is responsible for ensuring tagOrDigest uses the expected format.
 func (s *dockerImageSource) fetchManifest(ctx context.Context, tagOrDigest string) ([]byte, string, error) {
 	return s.c.fetchManifest(ctx, s.physicalRef, tagOrDigest)
 }
@@ -352,6 +357,9 @@ func (s *dockerImageSource) GetBlobAt(ctx context.Context, info types.BlobInfo,
 		return nil, nil, fmt.Errorf("external URLs not supported with GetBlobAt")
 	}
 
+	if err := info.Digest.Validate(); err != nil { // Make sure info.Digest.String() does not contain any unexpected characters
+		return nil, nil, err
+	}
 	path := fmt.Sprintf(blobsPath, reference.Path(s.physicalRef.ref), info.Digest.String())
 	logrus.Debugf("Downloading %s", path)
 	res, err := s.c.makeRequest(ctx, http.MethodGet, path, headers, nil, v2Auth, nil)

docker/internal/tarfile/writer.go

@@ -142,6 +142,9 @@ func (w *Writer) writeLegacyMetadataLocked(layerDescriptors []manifest.Schema2De
 		}
 
 		// This chainID value matches the computation in docker/docker/layer.CreateChainID …
+		if err := l.Digest.Validate(); err != nil { // This should never fail on this code path, still: make sure the chainID computation is unambiguous.
+			return err
+		}
 		if chainID == "" {
 			chainID = l.Digest
 		} else {

openshift/openshift_src.go

@@ -109,6 +109,9 @@ func (s *openshiftImageSource) GetSignaturesWithFormat(ctx context.Context, inst
 		}
 		imageStreamImageName = s.imageStreamImageName
 	} else {
+		if err := instanceDigest.Validate(); err != nil { // Make sure instanceDigest.String() does not contain any unexpected characters
+			return nil, err
+		}
 		imageStreamImageName = instanceDigest.String()
 	}
 	image, err := s.client.getImage(ctx, imageStreamImageName)

pkg/blobcache/blobcache.go

@@ -78,12 +78,15 @@ func (b *BlobCache) DeleteImage(ctx context.Context, sys *types.SystemContext) e
 }
 
 // blobPath returns the path appropriate for storing a blob with digest.
-func (b *BlobCache) blobPath(digest digest.Digest, isConfig bool) string {
+func (b *BlobCache) blobPath(digest digest.Digest, isConfig bool) (string, error) {
+	if err := digest.Validate(); err != nil { // Make sure digest.String() does not contain any unexpected characters
+		return "", err
+	}
 	baseName := digest.String()
 	if isConfig {
 		baseName += ".config"
 	}
-	return filepath.Join(b.directory, baseName)
+	return filepath.Join(b.directory, baseName), nil
 }
 
 // findBlob checks if we have a blob for info in cache (whether a config or not)
@@ -95,7 +98,10 @@ func (b *BlobCache) findBlob(info types.BlobInfo) (string, int64, bool, error) {
 	}
 
 	for _, isConfig := range []bool{false, true} {
-		path := b.blobPath(info.Digest, isConfig)
+		path, err := b.blobPath(info.Digest, isConfig)
+		if err != nil {
+			return "", -1, false, err
+		}
 		fileInfo, err := os.Stat(path)
 		if err == nil && (info.Size == -1 || info.Size == fileInfo.Size()) {
 			return path, fileInfo.Size(), isConfig, nil

pkg/blobcache/dest.go

@@ -114,7 +114,10 @@ func (d *blobCacheDestination) saveStream(wg *sync.WaitGroup, decompressReader i
 	}
 
 	// Determine the name that we should give to the uncompressed copy of the blob.
-	decompressedFilename := d.reference.blobPath(digester.Digest(), isConfig)
+	decompressedFilename, err := d.reference.blobPath(digester.Digest(), isConfig)
+	if err != nil {
+		return
+	}
 	// Rename the temporary file.
 	if err := os.Rename(tempFile.Name(), decompressedFilename); err != nil {
 		logrus.Debugf("error renaming new decompressed copy of blob %q into place at %q: %v", digester.Digest().String(), decompressedFilename, err)
@@ -152,7 +155,10 @@ func (d *blobCacheDestination) PutBlobWithOptions(ctx context.Context, stream io
 	needToWait := false
 	compression := archive.Uncompressed
 	if inputInfo.Digest != "" {
-		filename := d.reference.blobPath(inputInfo.Digest, options.IsConfig)
+		filename, err2 := d.reference.blobPath(inputInfo.Digest, options.IsConfig)
+		if err2 != nil {
+			return private.UploadedBlob{}, err2
+		}
 		tempfile, err = os.CreateTemp(filepath.Dir(filename), filepath.Base(filename))
 		if err == nil {
 			stream = io.TeeReader(stream, tempfile)
@@ -281,7 +287,10 @@ func (d *blobCacheDestination) PutManifest(ctx context.Context, manifestBytes []
 	if err != nil {
 		logrus.Warnf("error digesting manifest %q: %v", string(manifestBytes), err)
 	} else {
-		filename := d.reference.blobPath(manifestDigest, false)
+		filename, err := d.reference.blobPath(manifestDigest, false)
+		if err != nil {
+			return err
+		}
 		if err = ioutils.AtomicWriteFile(filename, manifestBytes, 0600); err != nil {
 			logrus.Warnf("error saving manifest as %q: %v", filename, err)
 		}

pkg/blobcache/src.go

@@ -56,7 +56,10 @@ func (s *blobCacheSource) Close() error {
 
 func (s *blobCacheSource) GetManifest(ctx context.Context, instanceDigest *digest.Digest) ([]byte, string, error) {
 	if instanceDigest != nil {
-		filename := s.reference.blobPath(*instanceDigest, false)
+		filename, err := s.reference.blobPath(*instanceDigest, false)
+		if err != nil {
+			return nil, "", err
+		}
 		manifestBytes, err := os.ReadFile(filename)
 		if err == nil {
 			s.cacheHits++
@@ -136,8 +139,10 @@ func (s *blobCacheSource) LayerInfosForCopy(ctx context.Context, instanceDigest
 		replacedInfos := make([]types.BlobInfo, 0, len(infos))
 		for _, info := range infos {
 			var replaceDigest []byte
-			var err error
-			blobFile := s.reference.blobPath(info.Digest, false)
+			blobFile, err := s.reference.blobPath(info.Digest, false)
+			if err != nil {
+				return nil, err
+			}
 			var alternate string
 			switch s.reference.compress {
 			case types.Compress:
@@ -148,7 +153,10 @@ func (s *blobCacheSource) LayerInfosForCopy(ctx context.Context, instanceDigest
 				replaceDigest, err = os.ReadFile(alternate)
 			}
 			if err == nil && digest.Digest(replaceDigest).Validate() == nil {
-				alternate = s.reference.blobPath(digest.Digest(replaceDigest), false)
+				alternate, err = s.reference.blobPath(digest.Digest(replaceDigest), false)
+				if err != nil {
+					return nil, err
+				}
 				fileInfo, err := os.Stat(alternate)
 				if err == nil {
 					switch info.MediaType {

storage/storage_dest.go

@@ -1070,17 +1070,25 @@ func (s *storageImageDestination) Commit(ctx context.Context, unparsedToplevel t
 		if err != nil {
 			return fmt.Errorf("digesting top-level manifest: %w", err)
 		}
+		key, err := manifestBigDataKey(manifestDigest)
+		if err != nil {
+			return err
+		}
 		options.BigData = append(options.BigData, storage.ImageBigDataOption{
-			Key:    manifestBigDataKey(manifestDigest),
+			Key:    key,
 			Data:   toplevelManifest,
 			Digest: manifestDigest,
 		})
 	}
 	// Set up to save the image's manifest.  Allow looking it up by digest by using the key convention defined by the Store.
 	// Record the manifest twice: using a digest-specific key to allow references to that specific digest instance,
 	// and using storage.ImageDigestBigDataKey for future users that don’t specify any digest and for compatibility with older readers.
+	key, err := manifestBigDataKey(s.manifestDigest)
+	if err != nil {
+		return err
+	}
 	options.BigData = append(options.BigData, storage.ImageBigDataOption{
-		Key:    manifestBigDataKey(s.manifestDigest),
+		Key:    key,
 		Data:   s.manifest,
 		Digest: s.manifestDigest,
 	})

storage/storage_image.go

@@ -21,8 +21,11 @@ var (
 // manifestBigDataKey returns a key suitable for recording a manifest with the specified digest using storage.Store.ImageBigData and related functions.
 // If a specific manifest digest is explicitly requested by the user, the key returned by this function should be used preferably;
 // for compatibility, if a manifest is not available under this key, check also storage.ImageDigestBigDataKey
-func manifestBigDataKey(digest digest.Digest) string {
-	return storage.ImageDigestManifestBigDataNamePrefix + "-" + digest.String()
+func manifestBigDataKey(digest digest.Digest) (string, error) {
+	if err := digest.Validate(); err != nil { // Make sure info.Digest.String() uses the expected format and does not collide with other BigData keys.
+		return "", err
+	}
+	return storage.ImageDigestManifestBigDataNamePrefix + "-" + digest.String(), nil
 }
 
 // signatureBigDataKey returns a key suitable for recording the signatures associated with the manifest with the specified digest using storage.Store.ImageBigData and related functions.

storage/storage_reference.go

@@ -73,7 +73,10 @@ func multiArchImageMatchesSystemContext(store storage.Store, img *storage.Image,
 	// We don't need to care about storage.ImageDigestBigDataKey because
 	// manifests lists are only stored into storage by c/image versions
 	// that know about manifestBigDataKey, and only using that key.
-	key := manifestBigDataKey(manifestDigest)
+	key, err := manifestBigDataKey(manifestDigest)
+	if err != nil {
+		return false // This should never happen, manifestDigest comes from a reference.Digested, and that validates the format.
+	}
 	manifestBytes, err := store.ImageBigData(img.ID, key)
 	if err != nil {
 		return false
@@ -95,7 +98,10 @@ func multiArchImageMatchesSystemContext(store storage.Store, img *storage.Image,
 	if err != nil {
 		return false
 	}
-	key = manifestBigDataKey(chosenInstance)
+	key, err = manifestBigDataKey(chosenInstance)
+	if err != nil {
+		return false
+	}
 	_, err = store.ImageBigData(img.ID, key)
 	return err == nil // true if img.ID is based on chosenInstance.
 }

storage/storage_src.go

@@ -237,7 +237,10 @@ func (s *storageImageSource) getBlobAndLayerID(digest digest.Digest, layers []st
 // GetManifest() reads the image's manifest.
 func (s *storageImageSource) GetManifest(ctx context.Context, instanceDigest *digest.Digest) (manifestBlob []byte, mimeType string, err error) {
 	if instanceDigest != nil {
-		key := manifestBigDataKey(*instanceDigest)
+		key, err := manifestBigDataKey(*instanceDigest)
+		if err != nil {
+			return nil, "", err
+		}
 		blob, err := s.imageRef.transport.store.ImageBigData(s.image.ID, key)
 		if err != nil {
 			return nil, "", fmt.Errorf("reading manifest for image instance %q: %w", *instanceDigest, err)
@@ -249,7 +252,10 @@ func (s *storageImageSource) GetManifest(ctx context.Context, instanceDigest *di
 		// Prefer the manifest corresponding to the user-specified digest, if available.
 		if s.imageRef.named != nil {
 			if digested, ok := s.imageRef.named.(reference.Digested); ok {
-				key := manifestBigDataKey(digested.Digest())
+				key, err := manifestBigDataKey(digested.Digest())
+				if err != nil {
+					return nil, "", err
+				}
 				blob, err := s.imageRef.transport.store.ImageBigData(s.image.ID, key)
 				if err != nil && !os.IsNotExist(err) { // os.IsNotExist is true if the image exists but there is no data corresponding to key
 					return nil, "", err