Skip to content

Commit cebe647

Browse files
mtrmacmtrmac
committed
Quote various strings coming from untrusted sources
Typically, use %q instead of %s (or instead of "%s"), to expose various control characters and the like without interpreting them. This is not really comprehensive; the codebase makes no _general_ guarantee that any returned string values are free of control characters or other malicious/misleading metadata. Not even in returned "error" values (which can legitimately contain newlines, if nothing else). Signed-off-by: Miloslav Trmač <[email protected]>
1 parent 24ac26d commit cebe647

28 files changed

+73
-73
lines changed

‎copy/manifest.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func determineManifestConversion(in determineManifestConversionInputs) (manifest
7474
srcType := in.srcMIMEType
7575
normalizedSrcType := manifest.NormalizedMIMEType(srcType)
7676
if srcType != normalizedSrcType {
77-
logrus.Debugf("Source manifest MIME type %s, treating it as %s", srcType, normalizedSrcType)
77+
logrus.Debugf("Source manifest MIME type %q, treating it as %q", srcType, normalizedSrcType)
7878
srcType = normalizedSrcType
7979
}
8080

@@ -237,7 +237,7 @@ func (c *copier) determineListConversion(currentListMIMEType string, destSupport
237237
}
238238
}
239239

240-
logrus.Debugf("Manifest list has MIME type %s, ordered candidate list [%s]", currentListMIMEType, strings.Join(destSupportedMIMETypes, ", "))
240+
logrus.Debugf("Manifest list has MIME type %q, ordered candidate list [%s]", currentListMIMEType, strings.Join(destSupportedMIMETypes, ", "))
241241
if len(prioritizedTypes.list) == 0 {
242242
return "", nil, fmt.Errorf("destination does not support any supported manifest list types (%v)", manifest.SupportedListMIMETypes)
243243
}

‎docker/archive/reader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func (r *Reader) List() ([][]types.ImageReference, error) {
7878
}
7979
nt, ok := parsedTag.(reference.NamedTagged)
8080
if !ok {
81-
return nil, fmt.Errorf("Invalid tag %s (%s): does not contain a tag", tag, parsedTag.String())
81+
return nil, fmt.Errorf("Invalid tag %q (%s): does not contain a tag", tag, parsedTag.String())
8282
}
8383
ref, err := newReference(r.path, nt, -1, r.archive, nil)
8484
if err != nil {

‎docker/daemon/daemon_dest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func imageLoad(ctx context.Context, c *client.Client, reader *io.PipeReader) err
116116
return fmt.Errorf("parsing docker load progress: %w", err)
117117
}
118118
if msg.Error != nil {
119-
return fmt.Errorf("docker engine reported: %s", msg.Error.Message)
119+
return fmt.Errorf("docker engine reported: %q", msg.Error.Message)
120120
}
121121
}
122122
return nil // No error reported = success

‎docker/internal/tarfile/reader.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func (r *Reader) openTarComponent(componentPath string) (io.ReadCloser, error) {
231231
}
232232

233233
if !header.FileInfo().Mode().IsRegular() {
234-
return nil, fmt.Errorf("Error reading tar archive component %s: not a regular file", header.Name)
234+
return nil, fmt.Errorf("Error reading tar archive component %q: not a regular file", header.Name)
235235
}
236236
succeeded = true
237237
return &tarReadCloser{Reader: tarReader, backingFile: f}, nil
@@ -262,7 +262,7 @@ func findTarComponent(inputFile io.Reader, componentPath string) (*tar.Reader, *
262262
func (r *Reader) readTarComponent(path string, limit int) ([]byte, error) {
263263
file, err := r.openTarComponent(path)
264264
if err != nil {
265-
return nil, fmt.Errorf("loading tar component %s: %w", path, err)
265+
return nil, fmt.Errorf("loading tar component %q: %w", path, err)
266266
}
267267
defer file.Close()
268268
bytes, err := iolimits.ReadAtMost(file, limit)

‎docker/internal/tarfile/src.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,10 @@ func (s *Source) ensureCachedDataIsPresentPrivate() error {
9595
}
9696
var parsedConfig manifest.Schema2Image // There's a lot of info there, but we only really care about layer DiffIDs.
9797
if err := json.Unmarshal(configBytes, &parsedConfig); err != nil {
98-
return fmt.Errorf("decoding tar config %s: %w", tarManifest.Config, err)
98+
return fmt.Errorf("decoding tar config %q: %w", tarManifest.Config, err)
9999
}
100100
if parsedConfig.RootFS == nil {
101-
return fmt.Errorf("Invalid image config (rootFS is not set): %s", tarManifest.Config)
101+
return fmt.Errorf("Invalid image config (rootFS is not set): %q", tarManifest.Config)
102102
}
103103

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

186186
uncompressedSize := h.Size
187187
if isCompressed {
188188
uncompressedSize, err = io.Copy(io.Discard, uncompressedStream)
189189
if err != nil {
190-
return nil, fmt.Errorf("reading %s to find its size: %w", layerPath, err)
190+
return nil, fmt.Errorf("reading %q to find its size: %w", layerPath, err)
191191
}
192192
}
193193
li.size = uncompressedSize

‎docker/registries_d.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func loadAndMergeConfig(dirPath string) (*registryConfiguration, error) {
140140

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

150150
for nsName, nsConfig := range config.Docker { // includes config.Docker == nil
151151
if _, ok := mergedConfig.Docker[nsName]; ok {
152-
return nil, fmt.Errorf(`Error parsing signature storage configuration: "docker" namespace "%s" defined both in "%s" and "%s"`,
152+
return nil, fmt.Errorf(`Error parsing signature storage configuration: "docker" namespace %q defined both in %q and %q`,
153153
nsName, nsMergedFrom[nsName], configPath)
154154
}
155155
mergedConfig.Docker[nsName] = nsConfig

‎internal/image/manifest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func manifestInstanceFromBlob(ctx context.Context, sys *types.SystemContext, src
7676
case imgspecv1.MediaTypeImageIndex:
7777
return manifestOCI1FromImageIndex(ctx, sys, src, manblob)
7878
default: // Note that this may not be reachable, manifest.NormalizedMIMEType has a default for unknown values.
79-
return nil, fmt.Errorf("Unimplemented manifest MIME type %s", mt)
79+
return nil, fmt.Errorf("Unimplemented manifest MIME type %q", mt)
8080
}
8181
}
8282

‎internal/manifest/docker_schema2_list.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func (list *Schema2ListPublic) ChooseInstance(ctx *types.SystemContext) (digest.
164164
}
165165
}
166166
}
167-
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)
167+
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)
168168
}
169169

170170
// Serialize returns the list in a blob format.

‎internal/manifest/list.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,5 +129,5 @@ func ListFromBlob(manifest []byte, manifestMIMEType string) (List, error) {
129129
case DockerV2Schema1MediaType, DockerV2Schema1SignedMediaType, imgspecv1.MediaTypeImageManifest, DockerV2Schema2MediaType:
130130
return nil, fmt.Errorf("Treating single images as manifest lists is not implemented")
131131
}
132-
return nil, fmt.Errorf("Unimplemented manifest list MIME type %s (normalized as %s)", manifestMIMEType, normalized)
132+
return nil, fmt.Errorf("Unimplemented manifest list MIME type %q (normalized as %q)", manifestMIMEType, normalized)
133133
}

‎internal/manifest/oci_index.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func (index *OCI1IndexPublic) chooseInstance(ctx *types.SystemContext, preferGzi
260260
if bestMatch != nil {
261261
return bestMatch.digest, nil
262262
}
263-
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)
263+
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)
264264
}
265265

266266
func (index *OCI1Index) ChooseInstanceByCompression(ctx *types.SystemContext, preferGzip types.OptionalBool) (digest.Digest, error) {

‎manifest/common.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,15 @@ func compressionVariantMIMEType(variantTable []compressionMIMETypeSet, mimeType
6767
return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mimeType)}
6868
}
6969
if name != mtsUncompressed {
70-
return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("unknown compressed with algorithm %s variant for type %s", name, mimeType)}
70+
return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("unknown compressed with algorithm %s variant for type %q", name, mimeType)}
7171
}
7272
// We can't very well say “the idea of no compression is unknown”
7373
return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mimeType)}
7474
}
7575
if algorithm != nil {
76-
return "", fmt.Errorf("unsupported MIME type for compression: %s", mimeType)
76+
return "", fmt.Errorf("unsupported MIME type for compression: %q", mimeType)
7777
}
78-
return "", fmt.Errorf("unsupported MIME type for decompression: %s", mimeType)
78+
return "", fmt.Errorf("unsupported MIME type for decompression: %q", mimeType)
7979
}
8080

8181
// updatedMIMEType returns the result of applying edits in updated (MediaType, CompressionOperation) to

‎manifest/docker_schema1.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ func (m *Schema1) fixManifestLayers() error {
221221
m.History = slices.Delete(m.History, i, i+1)
222222
m.ExtractedV1Compatibility = slices.Delete(m.ExtractedV1Compatibility, i, i+1)
223223
} else if m.ExtractedV1Compatibility[i].Parent != m.ExtractedV1Compatibility[i+1].ID {
224-
return fmt.Errorf("Invalid parent ID. Expected %v, got %v", m.ExtractedV1Compatibility[i+1].ID, m.ExtractedV1Compatibility[i].Parent)
224+
return fmt.Errorf("Invalid parent ID. Expected %v, got %q", m.ExtractedV1Compatibility[i+1].ID, m.ExtractedV1Compatibility[i].Parent)
225225
}
226226
}
227227
return nil

‎manifest/manifest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,5 +166,5 @@ func FromBlob(manblob []byte, mt string) (Manifest, error) {
166166
return nil, fmt.Errorf("Treating manifest lists as individual manifests is not implemented")
167167
}
168168
// Note that this may not be reachable, NormalizedMIMEType has a default for unknown values.
169-
return nil, fmt.Errorf("Unimplemented manifest MIME type %s (normalized as %s)", mt, nmt)
169+
return nil, fmt.Errorf("Unimplemented manifest MIME type %q (normalized as %q)", mt, nmt)
170170
}

‎manifest/oci.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func (m *OCI1) UpdateLayerInfos(layerInfos []types.BlobInfo) error {
167167
// an error if the mediatype does not support encryption
168168
func getEncryptedMediaType(mediatype string) (string, error) {
169169
if slices.Contains(strings.Split(mediatype, "+")[1:], "encrypted") {
170-
return "", fmt.Errorf("unsupported mediaType: %v already encrypted", mediatype)
170+
return "", fmt.Errorf("unsupported mediaType: %q already encrypted", mediatype)
171171
}
172172
unsuffixedMediatype := strings.Split(mediatype, "+")[0]
173173
switch unsuffixedMediatype {
@@ -176,15 +176,15 @@ func getEncryptedMediaType(mediatype string) (string, error) {
176176
return mediatype + "+encrypted", nil
177177
}
178178

179-
return "", fmt.Errorf("unsupported mediaType to encrypt: %v", mediatype)
179+
return "", fmt.Errorf("unsupported mediaType to encrypt: %q", mediatype)
180180
}
181181

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

190190
return res, nil

‎oci/layout/oci_src.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,19 +182,19 @@ func (s *ociImageSource) getExternalBlob(ctx context.Context, urls []string) (io
182182
hasSupportedURL = true
183183
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
184184
if err != nil {
185-
errWrap = fmt.Errorf("fetching %s failed %s: %w", u, err.Error(), errWrap)
185+
errWrap = fmt.Errorf("fetching %q failed %s: %w", u, err.Error(), errWrap)
186186
continue
187187
}
188188

189189
resp, err := s.client.Do(req)
190190
if err != nil {
191-
errWrap = fmt.Errorf("fetching %s failed %s: %w", u, err.Error(), errWrap)
191+
errWrap = fmt.Errorf("fetching %q failed %s: %w", u, err.Error(), errWrap)
192192
continue
193193
}
194194

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

‎openshift/openshift-copies.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ func (rules *clientConfigLoadingRules) Load() (*clientcmdConfig, error) {
553553
continue
554554
}
555555
if err != nil {
556-
errlist = append(errlist, fmt.Errorf("loading config file \"%s\": %w", filename, err))
556+
errlist = append(errlist, fmt.Errorf("loading config file %q: %w", filename, err))
557557
continue
558558
}
559559

‎openshift/openshift.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func (c *openshiftClient) getImage(ctx context.Context, imageStreamImageName str
152152
func (c *openshiftClient) convertDockerImageReference(ref string) (string, error) {
153153
_, repo, gotRepo := strings.Cut(ref, "/")
154154
if !gotRepo {
155-
return "", fmt.Errorf("Invalid format of docker reference %s: missing '/'", ref)
155+
return "", fmt.Errorf("Invalid format of docker reference %q: missing '/'", ref)
156156
}
157157
return reference.Domain(c.ref.dockerReference) + "/" + repo, nil
158158
}

‎signature/docker.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ func VerifyImageManifestSignatureUsingKeyIdentityList(unverifiedSignature, unver
7676
validateSignedDockerReference: func(signedDockerReference string) error {
7777
signedRef, err := reference.ParseNormalizedNamed(signedDockerReference)
7878
if err != nil {
79-
return internal.NewInvalidSignatureError(fmt.Sprintf("Invalid docker reference %s in signature", signedDockerReference))
79+
return internal.NewInvalidSignatureError(fmt.Sprintf("Invalid docker reference %q in signature", signedDockerReference))
8080
}
8181
if signedRef.String() != expectedRef.String() {
82-
return internal.NewInvalidSignatureError(fmt.Sprintf("Docker reference %s does not match %s",
82+
return internal.NewInvalidSignatureError(fmt.Sprintf("Docker reference %q does not match %q",
8383
signedDockerReference, expectedDockerReference))
8484
}
8585
return nil

‎signature/fulcio_cert.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func (f *fulcioTrustRoot) verifyFulcioCertificateAtTime(relevantTime time.Time,
178178

179179
// == Validate the OIDC subject
180180
if !slices.Contains(untrustedCertificate.EmailAddresses, f.subjectEmail) {
181-
return nil, internal.NewInvalidSignatureError(fmt.Sprintf("Required email %s not found (got %#v)",
181+
return nil, internal.NewInvalidSignatureError(fmt.Sprintf("Required email %q not found (got %q)",
182182
f.subjectEmail,
183183
untrustedCertificate.EmailAddresses))
184184
}

‎signature/fulcio_cert_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ func TestFulcioTrustRootVerifyFulcioCertificateAtTime(t *testing.T) {
327327
Value: sansBytes,
328328
})
329329
},
330-
errorFragment: "Required email [email protected] not found",
330+
errorFragment: `Required email "[email protected]" not found`,
331331
},
332332
{ // Other completely unrecognized critical extensions still cause failures
333333
name: "Unhandled critical extension",
@@ -367,7 +367,7 @@ func TestFulcioTrustRootVerifyFulcioCertificateAtTime(t *testing.T) {
367367
fn: func(cert *x509.Certificate) {
368368
cert.EmailAddresses = nil
369369
},
370-
errorFragment: "Required email [email protected] not found",
370+
errorFragment: `Required email "[email protected]" not found`,
371371
},
372372
{
373373
name: "Multiple emails, one matches",
@@ -381,14 +381,14 @@ func TestFulcioTrustRootVerifyFulcioCertificateAtTime(t *testing.T) {
381381
fn: func(cert *x509.Certificate) {
382382
cert.EmailAddresses = []string{"[email protected]"}
383383
},
384-
errorFragment: "Required email [email protected] not found",
384+
errorFragment: `Required email "[email protected]" not found`,
385385
},
386386
{
387387
name: "Multiple emails, no matches",
388388
fn: func(cert *x509.Certificate) {
389389
cert.EmailAddresses = []string{"[email protected]", "[email protected]", "[email protected]"}
390390
},
391-
errorFragment: "Required email [email protected] not found",
391+
errorFragment: `Required email "[email protected]" not found`,
392392
},
393393
} {
394394
testLeafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)

‎signature/internal/json.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func ParanoidUnmarshalJSONObject(data []byte, fieldResolver func(string) any) er
3131
return JSONFormatError(err.Error())
3232
}
3333
if t != json.Delim('{') {
34-
return JSONFormatError(fmt.Sprintf("JSON object expected, got \"%s\"", t))
34+
return JSONFormatError(fmt.Sprintf("JSON object expected, got %#v", t))
3535
}
3636
for {
3737
t, err := dec.Token()
@@ -45,16 +45,16 @@ func ParanoidUnmarshalJSONObject(data []byte, fieldResolver func(string) any) er
4545
key, ok := t.(string)
4646
if !ok {
4747
// Coverage: This should never happen, dec.Token() rejects non-string-literals in this state.
48-
return JSONFormatError(fmt.Sprintf("Key string literal expected, got \"%s\"", t))
48+
return JSONFormatError(fmt.Sprintf("Key string literal expected, got %#v", t))
4949
}
5050
if seenKeys.Contains(key) {
51-
return JSONFormatError(fmt.Sprintf("Duplicate key \"%s\"", key))
51+
return JSONFormatError(fmt.Sprintf("Duplicate key %q", key))
5252
}
5353
seenKeys.Add(key)
5454

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

0 commit comments

Comments
 (0)