check bundle layer size before reading in remote.Bundle#4947
Conversation
Signed-off-by: alhudz <al.hudz.k@gmail.com>
steiza
left a comment
There was a problem hiding this comment.
Nice catch! I suppose if we have size limits, we should be enforcing them consistently. We do know that some folks have very large bundles that include SBOMs though that are > 200 MB. Of course Rekor v1 has size limits as well.
Regardless, I think we want to test this change in e2e_test.go instead of in the unit test.
| } | ||
|
|
||
| func TestBundleRejectsOversizeLayer(t *testing.T) { | ||
| ri := remote.Image |
There was a problem hiding this comment.
I don't think we want to be patching other modules as part of a unit test - maybe this would be better off in e2e_test.go where we have a container registry running?
There was a problem hiding this comment.
Moved it to e2e_test.go as TestBundleLayerSizeCap, so it runs against the real registry from reg(t) and drops the fake oversizeLayer / remoteImage patching. It signs a new-format bundle, then lowers COSIGN_MAX_ATTACHMENT_SIZE below the layer size and checks ociremote.Bundle returns the limit error. Fails on main (the read path is reached), passes with the cap in place.
Signed-off-by: alhudz <al.hudz.k@gmail.com>
|
On the large SBOM bundles: this just reuses the same |
|
gentle ping |
Repro: point
cosign verifyat an image whose.sig-style bundle reference resolves to an image with oneapplication/vnd.dev.sigstore.bundlelayer that declares (or expands to) a large size.GetBundles->ociremote.Bundlereads it before any validation.Cause:
remote.Bundlecallslayers[0].Uncompressed()+io.ReadAllwith no size cap, while the sibling registry readers (pkg/oci/internal/signature/layer.go,pkg/oci/remote/remote.go,pkg/oci/static/file.go) all gate onpayloadsize.CheckSizefirst, so a hostile registry can make cosign allocate unbounded memory on a layer it has not yet trusted.Fix: apply the same
payloadsize.CheckSize(uint64(layer.Size()))check before reading, honouringCOSIGN_MAX_ATTACHMENT_SIZElike the other layers do.Added a regression test that fails on
main(the read path is reached) and passes with the cap in place.