From ce08e529d339f39a8a98f59a3c6141f7c91a8d76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Tue, 8 Sep 2026 20:54:11 +0200 Subject: [PATCH] [client] Bound what one peer can announce to the file drop receiver validateOffer bounded the file count and inline text but only checked the sign of an announced size, and nothing limited the aggregate or how many offers a sender could keep open. Every announced byte is staged in the spool before delivery, so a peer decided how much of the receiver's disk to take: 512 files of 8 GiB were accepted unchallenged, and a flood of offers each raised its own consent prompt. Sizes are now capped per file and per offer, and a sender is held to a fixed number of open offers. The aggregate accumulates against the remaining headroom instead of summing first, because 512 files of 2^60 wrap an int64 back through zero and a plain sum would report such an offer as nil bytes. The count is of open offers, not lifetime ones, so settling one frees a slot. --- client/internal/filedrop/filedrop_test.go | 50 +++++++++++++++++++++++ client/internal/filedrop/offer.go | 15 +++++++ client/internal/filedrop/protocol.go | 12 ++++++ client/internal/filedrop/receiver.go | 15 +++++++ 4 files changed, 92 insertions(+) diff --git a/client/internal/filedrop/filedrop_test.go b/client/internal/filedrop/filedrop_test.go index 69c7aad81..92cffeccc 100644 --- a/client/internal/filedrop/filedrop_test.go +++ b/client/internal/filedrop/filedrop_test.go @@ -711,7 +711,57 @@ func TestValidateOffer(t *testing.T) { Name: "x", Kind: KindText, Text: strings.Repeat("a", MaxInlineTextSize+1), }}), "oversized inline text is invalid") + assert.Error(t, validateOffer([]FileMeta{{Name: "x", Size: MaxFileSize + 1}}), + "a file over the per-file limit is invalid") + + overAggregate := make([]FileMeta, 4) + for i := range overAggregate { + overAggregate[i] = FileMeta{Name: "x", Size: MaxOfferSize / 3} + } + assert.Error(t, validateOffer(overAggregate), "an offer over the aggregate limit is invalid") + + // 512 x 2^60 sums back through zero in an int64, so a plain accumulation + // would wave this through as a nil-byte offer. + wrapping := make([]FileMeta, MaxOfferFiles) + for i := range wrapping { + wrapping[i] = FileMeta{Name: "x", Size: 1 << 60} + } + assert.Error(t, validateOffer(wrapping), "sizes that overflow int64 must not wrap past the limit") + assert.NoError(t, validateOffer([]FileMeta{{Name: "x", Size: 10}})) + assert.NoError(t, validateOffer([]FileMeta{{Name: "x", Size: MaxFileSize}}), + "a file exactly at the limit is allowed") +} + +func TestOfferStoreBoundsOffersPerSender(t *testing.T) { + policy := NewPolicyStore(testProfile) + require.NoError(t, policy.Set(Policy{Mode: ModeAsk})) + + spool, err := NewSpool(t.TempDir()) + require.NoError(t, err) + + r := &receiver{offers: NewOfferStore(time.Minute), policy: policy, spool: spool} + req := OfferRequest{Files: []FileMeta{{Name: "a.bin", Size: 10}}} + + for i := range MaxSenderOffers { + _, err := r.submitOffer(senderIdentity{key: testPeer}, req) + require.NoErrorf(t, err, "offer %d must be accepted", i) + } + + _, err = r.submitOffer(senderIdentity{key: testPeer}, req) + require.ErrorIs(t, err, ErrRefused, "the sender must be capped once its offers are open") + + _, err = r.submitOffer(senderIdentity{key: PeerKey("other-peer")}, req) + require.NoError(t, err, "the cap must be per sender, not global") + + // Settling one frees a slot: the cap counts open offers, not lifetime ones. + open := r.offers.List() + require.NotEmpty(t, open) + _, ok := r.offers.Decide(open[0].ID, DecisionDeclined) + require.True(t, ok) + + _, err = r.submitOffer(senderIdentity{key: testPeer}, req) + require.NoError(t, err, "a settled offer must release its slot") } func TestStopIsIdempotent(t *testing.T) { diff --git a/client/internal/filedrop/offer.go b/client/internal/filedrop/offer.go index 3f28294ae..024d391f9 100644 --- a/client/internal/filedrop/offer.go +++ b/client/internal/filedrop/offer.go @@ -89,6 +89,21 @@ func (s *OfferStore) Get(sender PeerKey, id OfferID) (Offer, bool) { return entry.offer.clone(), true } +// LiveCount reports how many of one sender's offers are still open, so a +// sender cannot keep adding to the ones already awaiting a decision. +func (s *OfferStore) LiveCount(sender PeerKey) int { + s.mu.RLock() + defer s.mu.RUnlock() + + n := 0 + for _, entry := range s.offers { + if entry.offer.Sender == sender && !entry.offer.State.terminal() { + n++ + } + } + return n +} + // List returns snapshots of every tracked offer. func (s *OfferStore) List() []Offer { s.mu.RLock() diff --git a/client/internal/filedrop/protocol.go b/client/internal/filedrop/protocol.go index c2eb3c045..e6ec1c304 100644 --- a/client/internal/filedrop/protocol.go +++ b/client/internal/filedrop/protocol.go @@ -26,6 +26,18 @@ const MaxOfferFiles = 512 // MaxInlineTextSize bounds an inline text snippet, which is held in memory. const MaxInlineTextSize = 64 * 1024 +// MaxFileSize bounds one announced payload, and MaxOfferSize the whole offer. +// Every announced byte is staged in the spool before it is delivered, so +// without these one peer decides how much of the receiver's disk to consume. +const ( + MaxFileSize int64 = 100 << 30 + MaxOfferSize int64 = 200 << 30 +) + +// MaxSenderOffers bounds how many offers one sender may have outstanding, so a +// peer cannot flood the offer store or the consent prompts behind it. +const MaxSenderOffers = 16 + const maxOfferBodySize = 1 << 20 const ( diff --git a/client/internal/filedrop/receiver.go b/client/internal/filedrop/receiver.go index a4cea0335..6a391fb02 100644 --- a/client/internal/filedrop/receiver.go +++ b/client/internal/filedrop/receiver.go @@ -61,6 +61,10 @@ func (r *receiver) submitOffer(sender senderIdentity, req OfferRequest) (Offer, return Offer{}, ErrRefused } + if r.offers.LiveCount(sender.key) >= MaxSenderOffers { + return Offer{}, fmt.Errorf("%w: %d offers already open", ErrRefused, MaxSenderOffers) + } + senderName := sender.name if senderName == "" { senderName = req.SenderName @@ -266,6 +270,7 @@ func validateOffer(files []FileMeta) error { return fmt.Errorf("offer announces more than %d files", MaxOfferFiles) } + var total int64 for _, f := range files { if !f.Kind.valid() { return fmt.Errorf("unknown payload kind %s", f.Kind) @@ -279,6 +284,16 @@ func validateOffer(files []FileMeta) error { if f.Size < 0 { return fmt.Errorf("negative file size") } + if f.Size > MaxFileSize { + return fmt.Errorf("file %d bytes exceeds the %d byte limit", f.Size, MaxFileSize) + } + // Accumulated against the remaining headroom rather than summed first: + // 512 files of 2^60 wrap an int64 back through zero, so a plain sum + // would report a small total for an enormous offer. + if total > MaxOfferSize-f.Size { + return fmt.Errorf("offer exceeds the %d byte limit", MaxOfferSize) + } + total += f.Size } return nil }