Skip to content

Commit 8a68959

Browse files
authored
fix(api_server): cap callback request body size (#77) (#122)
* fix(api_server): cap callback request body size (#77) handleCallback binds JSON without limiting body size, allowing a malicious or malfunctioning peer to exhaust memory with oversized payloads (especially STUMP blobs). Bodies are now wrapped in http.MaxBytesReader; oversize requests return 413. The limit is configurable via Callback.MaxBodyBytes (default 16 MiB). Closes F-019. * test(api_server): authenticate the unknown-txid callback test PR #121 (issue #91) added TestHandleCallback_UnknownTxid_NoPhantomRow without the bearer header that PR #112 (issue #76) made mandatory on the callback endpoint, so the test was failing on every PR rebased onto main. Use the existing authedCallbackRequest helper.
1 parent 65b6959 commit 8a68959

5 files changed

Lines changed: 228 additions & 2 deletions

File tree

config.example.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,16 @@ health:
114114
# allow_private_ips: true if your deployment intentionally posts
115115
# callbacks to internal services (testing, k8s service DNS, intranet
116116
# webhooks). See finding F-017 / issue #75.
117+
#
118+
# max_body_bytes caps the size of an inbound POST
119+
# /api/v1/merkle-service/callback request body (JSON, with embedded STUMP
120+
# payload). Default 16 MiB is generous over a realistic STUMP delivery
121+
# while bounding worst-case memory if a peer is malicious or malfunctioning.
122+
# Oversize requests are rejected with 413 Payload Too Large. See finding
123+
# F-019 / issue #77.
117124
# callback:
118125
# allow_private_ips: false
126+
# max_body_bytes: 16777216 # 16 MiB
119127

120128
# propagation:
121129
# # Per-endpoint circuit-breaker for the datahub URL list. Endpoints that

config/config.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,25 @@ type CallbackConfig struct {
358358
// Operators running purely against internal services (testing rigs,
359359
// k8s service DNS, intranet webhooks) can set this true.
360360
AllowPrivateIPs bool `mapstructure:"allow_private_ips"`
361+
// MaxBodyBytes caps the size of an inbound POST
362+
// /api/v1/merkle-service/callback body, in bytes. The callback receives
363+
// JSON with embedded STUMP payloads (subtree merkle paths) that can be
364+
// genuinely large for busy subtrees, but unbounded reads let a malicious
365+
// or malfunctioning peer exhaust memory. Default 16 MiB is well over a
366+
// realistic STUMP delivery (~hundreds of KiB even for the largest
367+
// subtrees observed in production) while still bounding worst-case
368+
// memory use per request. A value <= 0 selects DefaultCallbackMaxBodyBytes.
369+
// Mitigates F-019 (callback JSON bodies and STUMP payloads are unbounded).
370+
MaxBodyBytes int64 `mapstructure:"max_body_bytes"`
361371
}
362372

373+
// DefaultCallbackMaxBodyBytes is the fallback value for
374+
// CallbackConfig.MaxBodyBytes when an operator leaves it unset (or sets a
375+
// non-positive value). 16 MiB is generous enough for the largest realistic
376+
// STUMP payload while still bounding memory against a hostile peer; see
377+
// F-019 for the threat model.
378+
const DefaultCallbackMaxBodyBytes int64 = 16 << 20
379+
363380
// TxValidatorConfig tunes the parallel batch validation pipeline. Parallelism
364381
// caps how many transactions are parsed and validated concurrently inside a
365382
// single flush window — bounded so a huge in-flight batch can't open more
@@ -503,6 +520,9 @@ func setDefaults() {
503520
// into a blind SSRF primitive against internal services and cloud
504521
// metadata endpoints.
505522
viper.SetDefault("callback.allow_private_ips", false)
523+
// Inbound callback body cap. 16 MiB headroom over realistic STUMP payloads
524+
// while bounding memory against a hostile or malfunctioning peer (F-019).
525+
viper.SetDefault("callback.max_body_bytes", DefaultCallbackMaxBodyBytes)
506526

507527
viper.SetDefault("storage_path", "~/.arcade")
508528
viper.SetDefault("chaintracks_server.enabled", true)

config/config_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,3 +213,15 @@ func TestResolveChaintracksNetwork(t *testing.T) {
213213
})
214214
}
215215
}
216+
217+
// TestDefaultCallbackMaxBodyBytes pins the default body cap for the inbound
218+
// callback receiver. Set to 16 MiB — comfortably over a realistic STUMP
219+
// delivery while bounding worst-case memory if a peer is malicious or
220+
// malfunctioning. Mitigates F-019 / issue #77; bumping this value should be
221+
// a deliberate, reviewed change.
222+
func TestDefaultCallbackMaxBodyBytes(t *testing.T) {
223+
const want int64 = 16 * 1024 * 1024
224+
if DefaultCallbackMaxBodyBytes != want {
225+
t.Errorf("DefaultCallbackMaxBodyBytes = %d, want %d", DefaultCallbackMaxBodyBytes, want)
226+
}
227+
}

services/api_server/handlers.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"go.uber.org/zap"
1818

1919
"github.com/bsv-blockchain/arcade/callbackurl"
20+
"github.com/bsv-blockchain/arcade/config"
2021
"github.com/bsv-blockchain/arcade/kafka"
2122
"github.com/bsv-blockchain/arcade/models"
2223
"github.com/bsv-blockchain/arcade/store"
@@ -226,6 +227,17 @@ func (s *Server) handleReady(c *gin.Context) {
226227
c.JSON(http.StatusOK, gin.H{"status": "ready"})
227228
}
228229

230+
// callbackMaxBodyBytes returns the configured upper bound on the request
231+
// body for POST /api/v1/merkle-service/callback, falling back to the
232+
// package default when the operator leaves the knob unset or sets a
233+
// non-positive value. Centralized so the handler and tests stay in sync.
234+
func (s *Server) callbackMaxBodyBytes() int64 {
235+
if n := s.cfg.Callback.MaxBodyBytes; n > 0 {
236+
return n
237+
}
238+
return config.DefaultCallbackMaxBodyBytes
239+
}
240+
229241
// handleCallback processes inbound callbacks from Merkle Service.
230242
// Uses CallbackMessage format with Type field.
231243
//
@@ -235,6 +247,12 @@ func (s *Server) handleReady(c *gin.Context) {
235247
// means a misconfigured deployment outside the supported envelope. We still
236248
// fail closed here as a defense-in-depth measure: an empty/missing bearer or
237249
// any mismatch is rejected with 401 before any callback processing runs.
250+
//
251+
// The request body is wrapped in http.MaxBytesReader before JSON-binding so
252+
// a malicious or malfunctioning peer can't exhaust memory by streaming an
253+
// unbounded JSON body (notably the embedded STUMP blob). Oversize bodies
254+
// produce a 413 Payload Too Large; the cap is configurable via
255+
// Callback.MaxBodyBytes (default 16 MiB). See F-019 / issue #77.
238256
func (s *Server) handleCallback(c *gin.Context) {
239257
// Bearer token validation — always enforced, never skipped on empty
240258
// configured token. subtle.ConstantTimeCompare removes the timing side
@@ -251,8 +269,20 @@ func (s *Server) handleCallback(c *gin.Context) {
251269
return
252270
}
253271

272+
// Cap the inbound body BEFORE JSON-binding. http.MaxBytesReader returns a
273+
// *http.MaxBytesError once the limit is exceeded, which we map to 413.
274+
// Other decode errors (malformed JSON, type mismatches) keep their
275+
// existing 400 mapping — the contract for legitimate malformed input
276+
// hasn't changed.
277+
c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, s.callbackMaxBodyBytes())
278+
254279
var msg models.CallbackMessage
255280
if err := c.ShouldBindJSON(&msg); err != nil {
281+
var maxErr *http.MaxBytesError
282+
if errors.As(err, &maxErr) {
283+
c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "request body too large"})
284+
return
285+
}
256286
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"})
257287
return
258288
}

services/api_server/handlers_test.go

Lines changed: 158 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"net/http"
1111
"net/http/httptest"
1212
"reflect"
13+
"strings"
1314
"sync"
1415
"testing"
1516
"time"
@@ -398,8 +399,7 @@ func TestHandleCallback_UnknownTxid_NoPhantomRow(t *testing.T) {
398399
}
399400
body := mustMarshalJSON(t, payload)
400401

401-
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/api/v1/merkle-service/callback", bytes.NewReader(body))
402-
req.Header.Set("Content-Type", "application/json")
402+
req := authedCallbackRequest(t, body)
403403
w := httptest.NewRecorder()
404404

405405
router.ServeHTTP(w, req)
@@ -1048,3 +1048,159 @@ func TestHandleCallback_RejectsUnauthenticated(t *testing.T) {
10481048
})
10491049
}
10501050
}
1051+
1052+
// setupServerWithCallbackLimit returns a Server / router pair with the
1053+
// callback body cap explicitly configured. Used by the F-019 body-cap tests
1054+
// so they can run against a small, predictable limit instead of the 16 MiB
1055+
// production default.
1056+
func setupServerWithCallbackLimit(broker *kafka.RecordingBroker, ms *mockStore, maxBytes int64) (*Server, *gin.Engine) {
1057+
gin.SetMode(gin.TestMode)
1058+
producer := kafka.NewProducer(broker)
1059+
srv := &Server{
1060+
cfg: &config.Config{
1061+
CallbackToken: testCallbackToken,
1062+
Callback: config.CallbackConfig{MaxBodyBytes: maxBytes},
1063+
},
1064+
logger: zap.NewNop(),
1065+
producer: producer,
1066+
store: ms,
1067+
}
1068+
router := gin.New()
1069+
srv.registerRoutes(router)
1070+
return srv, router
1071+
}
1072+
1073+
// TestHandleCallback_BodySizeLimit_UnderLimitSucceeds verifies that a
1074+
// callback whose serialized JSON sits comfortably under the configured cap
1075+
// is processed normally — guarding against an overly aggressive limit
1076+
// rejecting legitimate STUMP deliveries. Locks down half of the F-019 fix.
1077+
func TestHandleCallback_BodySizeLimit_UnderLimitSucceeds(t *testing.T) {
1078+
const limit = 64 * 1024 // 64 KiB
1079+
ms := &mockStore{}
1080+
_, router := setupServerWithCallbackLimit(&kafka.RecordingBroker{}, ms, limit)
1081+
1082+
// Build a STUMP payload sized so that the resulting JSON body — which
1083+
// hex-encodes the bytes via models.HexBytes — is just under the cap. Hex
1084+
// roughly doubles the byte count, so half the limit (less envelope
1085+
// overhead) leaves headroom for the JSON wrapper.
1086+
stumpBytes := make([]byte, limit/2-1024)
1087+
for i := range stumpBytes {
1088+
stumpBytes[i] = byte(i & 0xFF)
1089+
}
1090+
payload := models.CallbackMessage{
1091+
Type: models.CallbackStump,
1092+
BlockHash: "0000000000000000000000000000000000000000000000000000000000000001",
1093+
Stump: stumpBytes,
1094+
}
1095+
body := mustMarshalJSON(t, payload)
1096+
if int64(len(body)) >= limit {
1097+
t.Fatalf("test setup: body %d bytes is not under limit %d", len(body), limit)
1098+
}
1099+
1100+
req := authedCallbackRequest(t, body)
1101+
w := httptest.NewRecorder()
1102+
router.ServeHTTP(w, req)
1103+
1104+
if w.Code != http.StatusOK {
1105+
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
1106+
}
1107+
1108+
ms.mu.Lock()
1109+
stored := len(ms.stumps)
1110+
ms.mu.Unlock()
1111+
if stored != 1 {
1112+
t.Errorf("expected 1 stump stored on under-limit body, got %d", stored)
1113+
}
1114+
}
1115+
1116+
// TestHandleCallback_BodySizeLimit_OverLimitReturns413 verifies the F-019
1117+
// fix: an oversize callback POST is rejected with 413 Payload Too Large
1118+
// (not 400) before any decoding allocates the full body. Locks down the
1119+
// other half of the fix.
1120+
func TestHandleCallback_BodySizeLimit_OverLimitReturns413(t *testing.T) {
1121+
const limit = 64 * 1024 // 64 KiB
1122+
ms := &mockStore{}
1123+
_, router := setupServerWithCallbackLimit(&kafka.RecordingBroker{}, ms, limit)
1124+
1125+
// Construct a body whose raw byte length exceeds the cap. We embed it in
1126+
// a CallbackSeenOnNetwork payload (oversize TxIDs list) so the structure
1127+
// is still valid JSON — what we're testing is the size check, not parse
1128+
// behavior on garbage input.
1129+
huge := strings.Repeat("a", int(limit)+1024)
1130+
body := mustMarshalJSON(t, models.CallbackMessage{
1131+
Type: models.CallbackSeenOnNetwork,
1132+
TxIDs: []string{huge},
1133+
})
1134+
if int64(len(body)) <= limit {
1135+
t.Fatalf("test setup: body %d bytes is not over limit %d", len(body), limit)
1136+
}
1137+
1138+
req := authedCallbackRequest(t, body)
1139+
w := httptest.NewRecorder()
1140+
router.ServeHTTP(w, req)
1141+
1142+
if w.Code != http.StatusRequestEntityTooLarge {
1143+
t.Fatalf("expected 413, got %d: %s", w.Code, w.Body.String())
1144+
}
1145+
1146+
// Oversize bodies must short-circuit before the dispatch path runs.
1147+
if len(ms.updateStatusCalls) != 0 {
1148+
t.Errorf("oversize callback must not write to the store, got %d UpdateStatus calls", len(ms.updateStatusCalls))
1149+
}
1150+
}
1151+
1152+
// TestHandleCallback_BodySizeLimit_HugeStumpRejected covers the original
1153+
// F-019 scenario directly: a STUMP callback whose embedded payload is much
1154+
// larger than the configured cap returns 413 instead of allocating the
1155+
// entire body into memory.
1156+
func TestHandleCallback_BodySizeLimit_HugeStumpRejected(t *testing.T) {
1157+
const limit = 32 * 1024 // 32 KiB
1158+
ms := &mockStore{}
1159+
_, router := setupServerWithCallbackLimit(&kafka.RecordingBroker{}, ms, limit)
1160+
1161+
// 4 MiB STUMP — vastly larger than the cap, mimicking the unbounded
1162+
// payload that motivated F-019.
1163+
stumpBytes := make([]byte, 4*1024*1024)
1164+
for i := range stumpBytes {
1165+
stumpBytes[i] = byte(i & 0xFF)
1166+
}
1167+
body := mustMarshalJSON(t, models.CallbackMessage{
1168+
Type: models.CallbackStump,
1169+
BlockHash: "0000000000000000000000000000000000000000000000000000000000000002",
1170+
Stump: stumpBytes,
1171+
})
1172+
1173+
req := authedCallbackRequest(t, body)
1174+
w := httptest.NewRecorder()
1175+
router.ServeHTTP(w, req)
1176+
1177+
if w.Code != http.StatusRequestEntityTooLarge {
1178+
t.Fatalf("expected 413, got %d: %s", w.Code, w.Body.String())
1179+
}
1180+
ms.mu.Lock()
1181+
stored := len(ms.stumps)
1182+
ms.mu.Unlock()
1183+
if stored != 0 {
1184+
t.Errorf("oversize STUMP must not be persisted, got %d stored", stored)
1185+
}
1186+
}
1187+
1188+
// TestHandleCallback_BodySizeLimit_DefaultsToConfigConstant confirms that
1189+
// leaving Callback.MaxBodyBytes unset (or zero/negative) falls back to the
1190+
// package default, so a misconfiguration can never disable the cap entirely.
1191+
func TestHandleCallback_BodySizeLimit_DefaultsToConfigConstant(t *testing.T) {
1192+
srv := &Server{cfg: &config.Config{}}
1193+
if got := srv.callbackMaxBodyBytes(); got != config.DefaultCallbackMaxBodyBytes {
1194+
t.Errorf("zero value: got %d, want %d", got, config.DefaultCallbackMaxBodyBytes)
1195+
}
1196+
1197+
srv.cfg.Callback.MaxBodyBytes = -1
1198+
if got := srv.callbackMaxBodyBytes(); got != config.DefaultCallbackMaxBodyBytes {
1199+
t.Errorf("negative value: got %d, want %d", got, config.DefaultCallbackMaxBodyBytes)
1200+
}
1201+
1202+
srv.cfg.Callback.MaxBodyBytes = 1 << 10
1203+
if got := srv.callbackMaxBodyBytes(); got != 1<<10 {
1204+
t.Errorf("explicit value: got %d, want %d", got, 1<<10)
1205+
}
1206+
}

0 commit comments

Comments
 (0)