Skip to content

Commit

Permalink
Merge pull request #2408 from mtrmac/quote-strings
Browse files Browse the repository at this point in the history
Quote various strings coming from untrusted sources
  • Loading branch information
rhatdan authored May 13, 2024
2 parents 1a2c225 + cebe647 commit a979521
Show file tree
Hide file tree
Showing 28 changed files with 73 additions and 73 deletions.
4 changes: 2 additions & 2 deletions copy/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func determineManifestConversion(in determineManifestConversionInputs) (manifest
srcType := in.srcMIMEType
normalizedSrcType := manifest.NormalizedMIMEType(srcType)
if srcType != normalizedSrcType {
logrus.Debugf("Source manifest MIME type %s, treating it as %s", srcType, normalizedSrcType)
logrus.Debugf("Source manifest MIME type %q, treating it as %q", srcType, normalizedSrcType)
srcType = normalizedSrcType
}

Expand Down Expand Up @@ -237,7 +237,7 @@ func (c *copier) determineListConversion(currentListMIMEType string, destSupport
}
}

logrus.Debugf("Manifest list has MIME type %s, ordered candidate list [%s]", currentListMIMEType, strings.Join(destSupportedMIMETypes, ", "))
logrus.Debugf("Manifest list has MIME type %q, ordered candidate list [%s]", currentListMIMEType, strings.Join(destSupportedMIMETypes, ", "))
if len(prioritizedTypes.list) == 0 {
return "", nil, fmt.Errorf("destination does not support any supported manifest list types (%v)", manifest.SupportedListMIMETypes)
}
Expand Down
2 changes: 1 addition & 1 deletion docker/archive/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (r *Reader) List() ([][]types.ImageReference, error) {
}
nt, ok := parsedTag.(reference.NamedTagged)
if !ok {
return nil, fmt.Errorf("Invalid tag %s (%s): does not contain a tag", tag, parsedTag.String())
return nil, fmt.Errorf("Invalid tag %q (%s): does not contain a tag", tag, parsedTag.String())
}
ref, err := newReference(r.path, nt, -1, r.archive, nil)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion docker/daemon/daemon_dest.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func imageLoad(ctx context.Context, c *client.Client, reader *io.PipeReader) err
return fmt.Errorf("parsing docker load progress: %w", err)
}
if msg.Error != nil {
return fmt.Errorf("docker engine reported: %s", msg.Error.Message)
return fmt.Errorf("docker engine reported: %q", msg.Error.Message)
}
}
return nil // No error reported = success
Expand Down
4 changes: 2 additions & 2 deletions docker/internal/tarfile/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func (r *Reader) openTarComponent(componentPath string) (io.ReadCloser, error) {
}

if !header.FileInfo().Mode().IsRegular() {
return nil, fmt.Errorf("Error reading tar archive component %s: not a regular file", header.Name)
return nil, fmt.Errorf("Error reading tar archive component %q: not a regular file", header.Name)
}
succeeded = true
return &tarReadCloser{Reader: tarReader, backingFile: f}, nil
Expand Down Expand Up @@ -262,7 +262,7 @@ func findTarComponent(inputFile io.Reader, componentPath string) (*tar.Reader, *
func (r *Reader) readTarComponent(path string, limit int) ([]byte, error) {
file, err := r.openTarComponent(path)
if err != nil {
return nil, fmt.Errorf("loading tar component %s: %w", path, err)
return nil, fmt.Errorf("loading tar component %q: %w", path, err)
}
defer file.Close()
bytes, err := iolimits.ReadAtMost(file, limit)
Expand Down
10 changes: 5 additions & 5 deletions docker/internal/tarfile/src.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ func (s *Source) ensureCachedDataIsPresentPrivate() error {
}
var parsedConfig manifest.Schema2Image // There's a lot of info there, but we only really care about layer DiffIDs.
if err := json.Unmarshal(configBytes, &parsedConfig); err != nil {
return fmt.Errorf("decoding tar config %s: %w", tarManifest.Config, err)
return fmt.Errorf("decoding tar config %q: %w", tarManifest.Config, err)
}
if parsedConfig.RootFS == nil {
return fmt.Errorf("Invalid image config (rootFS is not set): %s", tarManifest.Config)
return fmt.Errorf("Invalid image config (rootFS is not set): %q", tarManifest.Config)
}

knownLayers, err := s.prepareLayerData(tarManifest, &parsedConfig)
Expand Down Expand Up @@ -144,7 +144,7 @@ func (s *Source) prepareLayerData(tarManifest *ManifestItem, parsedConfig *manif
}
layerPath := path.Clean(tarManifest.Layers[i])
if _, ok := unknownLayerSizes[layerPath]; ok {
return nil, fmt.Errorf("Layer tarfile %s used for two different DiffID values", layerPath)
return nil, fmt.Errorf("Layer tarfile %q used for two different DiffID values", layerPath)
}
li := &layerInfo{ // A new element in each iteration
path: layerPath,
Expand Down Expand Up @@ -179,15 +179,15 @@ func (s *Source) prepareLayerData(tarManifest *ManifestItem, parsedConfig *manif
// the slower method of checking if it's compressed.
uncompressedStream, isCompressed, err := compression.AutoDecompress(t)
if err != nil {
return nil, fmt.Errorf("auto-decompressing %s to determine its size: %w", layerPath, err)
return nil, fmt.Errorf("auto-decompressing %q to determine its size: %w", layerPath, err)
}
defer uncompressedStream.Close()

uncompressedSize := h.Size
if isCompressed {
uncompressedSize, err = io.Copy(io.Discard, uncompressedStream)
if err != nil {
return nil, fmt.Errorf("reading %s to find its size: %w", layerPath, err)
return nil, fmt.Errorf("reading %q to find its size: %w", layerPath, err)
}
}
li.size = uncompressedSize
Expand Down
4 changes: 2 additions & 2 deletions docker/registries_d.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) {

if config.DefaultDocker != nil {
if mergedConfig.DefaultDocker != nil {
return nil, fmt.Errorf(`Error parsing signature storage configuration: "default-docker" defined both in "%s" and "%s"`,
return nil, fmt.Errorf(`Error parsing signature storage configuration: "default-docker" defined both in %q and %q`,
dockerDefaultMergedFrom, configPath)
}
mergedConfig.DefaultDocker = config.DefaultDocker
Expand All @@ -149,7 +149,7 @@ func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) {

for nsName, nsConfig := range config.Docker { // includes config.Docker == nil
if _, ok := mergedConfig.Docker[nsName]; ok {
return nil, fmt.Errorf(`Error parsing signature storage configuration: "docker" namespace "%s" defined both in "%s" and "%s"`,
return nil, fmt.Errorf(`Error parsing signature storage configuration: "docker" namespace %q defined both in %q and %q`,
nsName, nsMergedFrom[nsName], configPath)
}
mergedConfig.Docker[nsName] = nsConfig
Expand Down
2 changes: 1 addition & 1 deletion internal/image/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func manifestInstanceFromBlob(ctx context.Context, sys *types.SystemContext, src
case imgspecv1.MediaTypeImageIndex:
return manifestOCI1FromImageIndex(ctx, sys, src, manblob)
default: // Note that this may not be reachable, manifest.NormalizedMIMEType has a default for unknown values.
return nil, fmt.Errorf("Unimplemented manifest MIME type %s", mt)
return nil, fmt.Errorf("Unimplemented manifest MIME type %q", mt)
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/manifest/docker_schema2_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func (list *Schema2ListPublic) ChooseInstance(ctx *types.SystemContext) (digest.
}
}
}
return "", fmt.Errorf("no image found in manifest list for architecture %s, variant %q, OS %s", wantedPlatforms[0].Architecture, wantedPlatforms[0].Variant, wantedPlatforms[0].OS)
return "", fmt.Errorf("no image found in manifest list for architecture %q, variant %q, OS %q", wantedPlatforms[0].Architecture, wantedPlatforms[0].Variant, wantedPlatforms[0].OS)
}

// Serialize returns the list in a blob format.
Expand Down
2 changes: 1 addition & 1 deletion internal/manifest/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,5 +129,5 @@ func ListFromBlob(manifest []byte, manifestMIMEType string) (List, error) {
case DockerV2Schema1MediaType, DockerV2Schema1SignedMediaType, imgspecv1.MediaTypeImageManifest, DockerV2Schema2MediaType:
return nil, fmt.Errorf("Treating single images as manifest lists is not implemented")
}
return nil, fmt.Errorf("Unimplemented manifest list MIME type %s (normalized as %s)", manifestMIMEType, normalized)
return nil, fmt.Errorf("Unimplemented manifest list MIME type %q (normalized as %q)", manifestMIMEType, normalized)
}
2 changes: 1 addition & 1 deletion internal/manifest/oci_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func (index *OCI1IndexPublic) chooseInstance(ctx *types.SystemContext, preferGzi
if bestMatch != nil {
return bestMatch.digest, nil
}
return "", fmt.Errorf("no image found in image index for architecture %s, variant %q, OS %s", wantedPlatforms[0].Architecture, wantedPlatforms[0].Variant, wantedPlatforms[0].OS)
return "", fmt.Errorf("no image found in image index for architecture %q, variant %q, OS %q", wantedPlatforms[0].Architecture, wantedPlatforms[0].Variant, wantedPlatforms[0].OS)
}

func (index *OCI1Index) ChooseInstanceByCompression(ctx *types.SystemContext, preferGzip types.OptionalBool) (digest.Digest, error) {
Expand Down
6 changes: 3 additions & 3 deletions manifest/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ func compressionVariantMIMEType(variantTable []compressionMIMETypeSet, mimeType
return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mimeType)}
}
if name != mtsUncompressed {
return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("unknown compressed with algorithm %s variant for type %s", name, mimeType)}
return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("unknown compressed with algorithm %s variant for type %q", name, mimeType)}
}
// We can't very well say “the idea of no compression is unknown”
return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mimeType)}
}
if algorithm != nil {
return "", fmt.Errorf("unsupported MIME type for compression: %s", mimeType)
return "", fmt.Errorf("unsupported MIME type for compression: %q", mimeType)
}
return "", fmt.Errorf("unsupported MIME type for decompression: %s", mimeType)
return "", fmt.Errorf("unsupported MIME type for decompression: %q", mimeType)
}

// updatedMIMEType returns the result of applying edits in updated (MediaType, CompressionOperation) to
Expand Down
2 changes: 1 addition & 1 deletion manifest/docker_schema1.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func (m *Schema1) fixManifestLayers() error {
m.History = slices.Delete(m.History, i, i+1)
m.ExtractedV1Compatibility = slices.Delete(m.ExtractedV1Compatibility, i, i+1)
} else if m.ExtractedV1Compatibility[i].Parent != m.ExtractedV1Compatibility[i+1].ID {
return fmt.Errorf("Invalid parent ID. Expected %v, got %v", m.ExtractedV1Compatibility[i+1].ID, m.ExtractedV1Compatibility[i].Parent)
return fmt.Errorf("Invalid parent ID. Expected %v, got %q", m.ExtractedV1Compatibility[i+1].ID, m.ExtractedV1Compatibility[i].Parent)
}
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,5 +166,5 @@ func FromBlob(manblob []byte, mt string) (Manifest, error) {
return nil, fmt.Errorf("Treating manifest lists as individual manifests is not implemented")
}
// Note that this may not be reachable, NormalizedMIMEType has a default for unknown values.
return nil, fmt.Errorf("Unimplemented manifest MIME type %s (normalized as %s)", mt, nmt)
return nil, fmt.Errorf("Unimplemented manifest MIME type %q (normalized as %q)", mt, nmt)
}
6 changes: 3 additions & 3 deletions manifest/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (m *OCI1) UpdateLayerInfos(layerInfos []types.BlobInfo) error {
// an error if the mediatype does not support encryption
func getEncryptedMediaType(mediatype string) (string, error) {
if slices.Contains(strings.Split(mediatype, "+")[1:], "encrypted") {
return "", fmt.Errorf("unsupported mediaType: %v already encrypted", mediatype)
return "", fmt.Errorf("unsupported mediaType: %q already encrypted", mediatype)
}
unsuffixedMediatype := strings.Split(mediatype, "+")[0]
switch unsuffixedMediatype {
Expand All @@ -176,15 +176,15 @@ func getEncryptedMediaType(mediatype string) (string, error) {
return mediatype + "+encrypted", nil
}

return "", fmt.Errorf("unsupported mediaType to encrypt: %v", mediatype)
return "", fmt.Errorf("unsupported mediaType to encrypt: %q", mediatype)
}

// getDecryptedMediaType will return the mediatype to its encrypted counterpart and return
// an error if the mediatype does not support decryption
func getDecryptedMediaType(mediatype string) (string, error) {
res, ok := strings.CutSuffix(mediatype, "+encrypted")
if !ok {
return "", fmt.Errorf("unsupported mediaType to decrypt: %v", mediatype)
return "", fmt.Errorf("unsupported mediaType to decrypt: %q", mediatype)
}

return res, nil
Expand Down
6 changes: 3 additions & 3 deletions oci/layout/oci_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,19 +182,19 @@ func (s *ociImageSource) getExternalBlob(ctx context.Context, urls []string) (io
hasSupportedURL = true
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
if err != nil {
errWrap = fmt.Errorf("fetching %s failed %s: %w", u, err.Error(), errWrap)
errWrap = fmt.Errorf("fetching %q failed %s: %w", u, err.Error(), errWrap)
continue
}

resp, err := s.client.Do(req)
if err != nil {
errWrap = fmt.Errorf("fetching %s failed %s: %w", u, err.Error(), errWrap)
errWrap = fmt.Errorf("fetching %q failed %s: %w", u, err.Error(), errWrap)
continue
}

if resp.StatusCode != http.StatusOK {
resp.Body.Close()
errWrap = fmt.Errorf("fetching %s failed, response code not 200: %w", u, errWrap)
errWrap = fmt.Errorf("fetching %q failed, response code not 200: %w", u, errWrap)
continue
}

Expand Down
2 changes: 1 addition & 1 deletion openshift/openshift-copies.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ func (rules *clientConfigLoadingRules) Load() (*clientcmdConfig, error) {
continue
}
if err != nil {
errlist = append(errlist, fmt.Errorf("loading config file \"%s\": %w", filename, err))
errlist = append(errlist, fmt.Errorf("loading config file %q: %w", filename, err))
continue
}

Expand Down
2 changes: 1 addition & 1 deletion openshift/openshift.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (c *openshiftClient) getImage(ctx context.Context, imageStreamImageName str
func (c *openshiftClient) convertDockerImageReference(ref string) (string, error) {
_, repo, gotRepo := strings.Cut(ref, "/")
if !gotRepo {
return "", fmt.Errorf("Invalid format of docker reference %s: missing '/'", ref)
return "", fmt.Errorf("Invalid format of docker reference %q: missing '/'", ref)
}
return reference.Domain(c.ref.dockerReference) + "/" + repo, nil
}
Expand Down
4 changes: 2 additions & 2 deletions signature/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ func VerifyImageManifestSignatureUsingKeyIdentityList(unverifiedSignature, unver
validateSignedDockerReference: func(signedDockerReference string) error {
signedRef, err := reference.ParseNormalizedNamed(signedDockerReference)
if err != nil {
return internal.NewInvalidSignatureError(fmt.Sprintf("Invalid docker reference %s in signature", signedDockerReference))
return internal.NewInvalidSignatureError(fmt.Sprintf("Invalid docker reference %q in signature", signedDockerReference))
}
if signedRef.String() != expectedRef.String() {
return internal.NewInvalidSignatureError(fmt.Sprintf("Docker reference %s does not match %s",
return internal.NewInvalidSignatureError(fmt.Sprintf("Docker reference %q does not match %q",
signedDockerReference, expectedDockerReference))
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion signature/fulcio_cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func (f *fulcioTrustRoot) verifyFulcioCertificateAtTime(relevantTime time.Time,

// == Validate the OIDC subject
if !slices.Contains(untrustedCertificate.EmailAddresses, f.subjectEmail) {
return nil, internal.NewInvalidSignatureError(fmt.Sprintf("Required email %s not found (got %#v)",
return nil, internal.NewInvalidSignatureError(fmt.Sprintf("Required email %q not found (got %q)",
f.subjectEmail,
untrustedCertificate.EmailAddresses))
}
Expand Down
8 changes: 4 additions & 4 deletions signature/fulcio_cert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ func TestFulcioTrustRootVerifyFulcioCertificateAtTime(t *testing.T) {
Value: sansBytes,
})
},
errorFragment: "Required email [email protected] not found",
errorFragment: `Required email "[email protected]" not found`,
},
{ // Other completely unrecognized critical extensions still cause failures
name: "Unhandled critical extension",
Expand Down Expand Up @@ -367,7 +367,7 @@ func TestFulcioTrustRootVerifyFulcioCertificateAtTime(t *testing.T) {
fn: func(cert *x509.Certificate) {
cert.EmailAddresses = nil
},
errorFragment: "Required email [email protected] not found",
errorFragment: `Required email "[email protected]" not found`,
},
{
name: "Multiple emails, one matches",
Expand All @@ -381,14 +381,14 @@ func TestFulcioTrustRootVerifyFulcioCertificateAtTime(t *testing.T) {
fn: func(cert *x509.Certificate) {
cert.EmailAddresses = []string{"[email protected]"}
},
errorFragment: "Required email [email protected] not found",
errorFragment: `Required email "[email protected]" not found`,
},
{
name: "Multiple emails, no matches",
fn: func(cert *x509.Certificate) {
cert.EmailAddresses = []string{"[email protected]", "[email protected]", "[email protected]"}
},
errorFragment: "Required email [email protected] not found",
errorFragment: `Required email "[email protected]" not found`,
},
} {
testLeafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
Expand Down
10 changes: 5 additions & 5 deletions signature/internal/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func ParanoidUnmarshalJSONObject(data []byte, fieldResolver func(string) any) er
return JSONFormatError(err.Error())
}
if t != json.Delim('{') {
return JSONFormatError(fmt.Sprintf("JSON object expected, got \"%s\"", t))
return JSONFormatError(fmt.Sprintf("JSON object expected, got %#v", t))
}
for {
t, err := dec.Token()
Expand All @@ -45,16 +45,16 @@ func ParanoidUnmarshalJSONObject(data []byte, fieldResolver func(string) any) er
key, ok := t.(string)
if !ok {
// Coverage: This should never happen, dec.Token() rejects non-string-literals in this state.
return JSONFormatError(fmt.Sprintf("Key string literal expected, got \"%s\"", t))
return JSONFormatError(fmt.Sprintf("Key string literal expected, got %#v", t))
}
if seenKeys.Contains(key) {
return JSONFormatError(fmt.Sprintf("Duplicate key \"%s\"", key))
return JSONFormatError(fmt.Sprintf("Duplicate key %q", key))
}
seenKeys.Add(key)

valuePtr := fieldResolver(key)
if valuePtr == nil {
return JSONFormatError(fmt.Sprintf("Unknown key \"%s\"", key))
return JSONFormatError(fmt.Sprintf("Unknown key %q", key))
}
// This works like json.Unmarshal, in particular it allows us to implement UnmarshalJSON to implement strict parsing of the field value.
if err := dec.Decode(valuePtr); err != nil {
Expand Down Expand Up @@ -83,7 +83,7 @@ func ParanoidUnmarshalJSONObjectExactFields(data []byte, exactFields map[string]
}
for key := range exactFields {
if !seenKeys.Contains(key) {
return JSONFormatError(fmt.Sprintf(`Key "%s" missing in a JSON object`, key))
return JSONFormatError(fmt.Sprintf(`Key %q missing in a JSON object`, key))
}
}
return nil
Expand Down
Loading

0 comments on commit a979521

Please sign in to comment.