sigstore / cosign

Code signing and transparency for containers and binaries
Apache License 2.0
4.41k stars 543 forks source link

Memory Leak identified in cosign verify flow #3642

Closed Mukuls77 closed 6 months ago

Mukuls77 commented 6 months ago

Description

We are using cosign as a library in our code. when we execute cosign libraries to verify the images for signature, we identify a memory increase. After memory profiling of the code we found the issue in the file/function https://github.com/sigstore/cosign/blob/6206f5af392ccefcc6a6cbb167bbccc833b46ed4/pkg/oci/internal/signature/layer.go#L60

The main problem identified is that in this function defer clause is missing to close the file stream.

When we looked similar functionality elsewhere in cosign code , we found that defer clause is present. few examples are https://github.com/sigstore/cosign/blob/6206f5af392ccefcc6a6cbb167bbccc833b46ed4/pkg/oci/remote/remote.go#L228 and https://github.com/sigstore/cosign/blob/6206f5af392ccefcc6a6cbb167bbccc833b46ed4/pkg/oci/static/file.go#L84 in these functions the file stream is getting closed but seems to be missing in pkg/oci/internal/signature/layer.go

The main change suggested is to add defer clause which is missing in this function.

// Payload implements oci.Signature func (s *sigLayer) Payload() ([]byte, error) { // Compressed is a misnomer here, we just want the raw bytes from the registry. r, err := s.Layer.Compressed() if err != nil { return nil, err } payload, err := io.ReadAll(r) if err != nil { return nil, err } defer r.Close() return payload, nil }

We applied the patch locally and found the memory leak is solved.

Without Patch

image

With Patch image

Version

The issue is identified in release 2.2.3 but it is previous in previous release also