Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api: Limit request body size to 10MB #5246

Merged
merged 5 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion daemon/algod/api/server/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ const (
apiV1Tag = "/v1"
// TokenHeader is the header where we put the token.
TokenHeader = "X-Algo-API-Token"
// maxRequestBodyBytes is the maximum request body size that we allow in our APIs.
maxRequestBodyBytes = "10MB"
)

// wrapCtx passes a common context to each request without a global variable.
Expand Down Expand Up @@ -90,7 +92,9 @@ func NewRouter(logger logging.Logger, node APINodeInterface, shutdown <-chan str
middleware.RemoveTrailingSlash())
e.Use(
middlewares.MakeLogger(logger),
middlewares.MakeCORS(TokenHeader))
middlewares.MakeCORS(TokenHeader),
middleware.BodyLimit(maxRequestBodyBytes),
)

// Request Context
ctx := lib.ReqContext{Node: node, Log: logger, Shutdown: shutdown}
Expand Down
18 changes: 10 additions & 8 deletions daemon/algod/api/server/v2/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,16 @@ import (
"github.com/algorand/go-algorand/stateproof"
)

// max compiled teal program is currently 8k
// MaxTealSourceBytes sets a size limit for TEAL source programs for requests
// Max TEAL program size is currently 8k
// but we allow for comments, spacing, and repeated consts
// in the source teal, allow up to 200kb
const maxTealSourceBytes = 200_000
// in the source TEAL, so we allow up to 200KB
const MaxTealSourceBytes = 200_000

// MaxTealDryrunBytes sets a size limit for dryrun requests
// With the ability to hold unlimited assets DryrunRequests can
// become quite large, allow up to 1mb
const maxTealDryrunBytes = 1_000_000
// become quite large, so we allow up to 1MB
const MaxTealDryrunBytes = 1_000_000

// Handlers is an implementation to the V2 route handler interface defined by the generated code.
type Handlers struct {
Expand Down Expand Up @@ -995,7 +997,7 @@ func (v2 *Handlers) TealDryrun(ctx echo.Context) error {
}
req := ctx.Request()
buf := new(bytes.Buffer)
req.Body = http.MaxBytesReader(nil, req.Body, maxTealDryrunBytes)
req.Body = http.MaxBytesReader(nil, req.Body, MaxTealDryrunBytes)
_, err := buf.ReadFrom(ctx.Request().Body)
if err != nil {
return badRequest(ctx, err, err.Error(), v2.Log)
Expand Down Expand Up @@ -1528,7 +1530,7 @@ func (v2 *Handlers) TealCompile(ctx echo.Context, params model.TealCompileParams
}

buf := new(bytes.Buffer)
ctx.Request().Body = http.MaxBytesReader(nil, ctx.Request().Body, maxTealSourceBytes)
ctx.Request().Body = http.MaxBytesReader(nil, ctx.Request().Body, MaxTealSourceBytes)
_, err = buf.ReadFrom(ctx.Request().Body)
if err != nil {
return badRequest(ctx, err, err.Error(), v2.Log)
Expand Down Expand Up @@ -1645,7 +1647,7 @@ func (v2 *Handlers) TealDisassemble(ctx echo.Context) error {
return ctx.String(http.StatusNotFound, "/teal/disassemble was not enabled in the configuration file by setting the EnableDeveloperAPI to true")
}
buf := new(bytes.Buffer)
ctx.Request().Body = http.MaxBytesReader(nil, ctx.Request().Body, maxTealSourceBytes)
ctx.Request().Body = http.MaxBytesReader(nil, ctx.Request().Body, MaxTealSourceBytes)
_, err := buf.ReadFrom(ctx.Request().Body)
if err != nil {
return badRequest(ctx, err, err.Error(), v2.Log)
Expand Down
18 changes: 17 additions & 1 deletion daemon/algod/api/server/v2/test/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -1263,6 +1264,11 @@ func TestTealDisassemble(t *testing.T) {
// Test bad program.
badProgram := []byte{1, 99}
tealDisassembleTest(t, badProgram, 400, "invalid opcode", true)

// Create a program with MaxTealSourceBytes+1 bytes
// This should fail inside the handler when reading the bytes from the request body.
largeProgram := []byte(strings.Repeat("a", v2.MaxTealSourceBytes+1))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

tealDisassembleTest(t, largeProgram, 400, "http: request body too large", true)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are affected by a smaller size limit (200KB instead of 10MB) defined by MaxTealSourceBytes and will trigger the error here:

return badRequest(ctx, err, err.Error(), v2.Log)

}

func tealDryrunTest(
Expand Down Expand Up @@ -1309,6 +1315,12 @@ func tealDryrunTest(
messages := *response.Txns[0].AppCallMessages
require.GreaterOrEqual(t, len(messages), 1)
require.Equal(t, expResult, messages[len(messages)-1])
} else if rec.Code == 400 {
var response model.ErrorResponse
data := rec.Body.Bytes()
err = protocol.DecodeJSON(data, &response)
require.NoError(t, err, string(data))
require.Contains(t, response.Message, expResult)
}
return
}
Expand Down Expand Up @@ -1374,7 +1386,7 @@ func TestTealDryrun(t *testing.T) {
tealDryrunTest(t, &gdr, "msgp", 404, "", false)

gdr.ProtocolVersion = "unk"
tealDryrunTest(t, &gdr, "json", 400, "", true)
tealDryrunTest(t, &gdr, "json", 400, "unsupported protocol version", true)
gdr.ProtocolVersion = ""

ddr := tealDryrunTest(t, &gdr, "json", 200, "PASS", true)
Expand All @@ -1387,6 +1399,10 @@ func TestTealDryrun(t *testing.T) {
tealDryrunTest(t, &gdr, "json", 200, "REJECT", true)
tealDryrunTest(t, &gdr, "msgp", 200, "REJECT", true)
tealDryrunTest(t, &gdr, "json", 404, "", false)

// This should fail inside the handler when reading the bytes from the request body.
gdr.ProtocolVersion = strings.Repeat("a", v2.MaxTealDryrunBytes+1)
tealDryrunTest(t, &gdr, "json", 400, "http: request body too large", true)
}

func TestAppendParticipationKeys(t *testing.T) {
Expand Down
14 changes: 12 additions & 2 deletions test/scripts/e2e_subs/e2e-app-simulate.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash

date '+app-simple-test start %Y%m%d_%H%M%S'
bbroder-algo marked this conversation as resolved.
Show resolved Hide resolved
date '+app-simulate-test start %Y%m%d_%H%M%S'

set -e
set -x
Expand All @@ -20,6 +20,16 @@ ACCOUNT=$(${gcmd} account list|awk '{ print $3 }')
CONST_TRUE="true"
CONST_FALSE="false"

# First, try to send an extremely large "transaction" in the request body.
# This should fail with a 413 error.
truncate -s 11MB toolarge.tx
RES=$(${gcmd} clerk simulate -t toolarge.tx 2>&1 || true)
algochoi marked this conversation as resolved.
Show resolved Hide resolved
EXPERROR="simulation error: HTTP 413 Request Entity Too Large:"
if [[ $RES != *"${EXPERROR}"* ]]; then
date '+app-simulate-test FAIL the simulate API should fail for request bodies exceeding 10MB %Y%m%d_%H%M%S'
false
fi

##############################################
# WE FIRST TEST TRANSACTION GROUP SIMULATION #
##############################################
Expand All @@ -29,7 +39,7 @@ ${gcmd} clerk send -a 10000 -f ${ACCOUNT} -t ${ACCOUNT} -o pay2.tx

cat pay1.tx pay2.tx | ${gcmd} clerk group -i - -o grouped.tx

# We first test transaction group simulation WITHOUT signatures
# We test transaction group simulation WITHOUT signatures
RES=$(${gcmd} clerk simulate -t grouped.tx)

if [[ $(echo "$RES" | jq '."would-succeed"') != $CONST_FALSE ]]; then
Expand Down