clawbot
fe9454f7b3
All checks were successful
check / check (push) Successful in 2m54s
MaxBodySize logged r.URL.Path untruncated at WARN, and routes.go registers it ahead of RequireAuth, so an unauthenticated POST /source/<8 KB>/edit with an oversize declared Content-Length wrote attacker-chosen text of attacker-chosen length into the operator's log, for the cost of a request with no body. The 2,560-byte per-line budget from #146 did not reach it: that budget lives in the access log's field capping and this is a separate slog call. The capping mechanism moves out of internal/middleware into internal/logfield so there is one budget and one implementation rather than a second ad-hoc truncation. Truncate and EncodedBytes are unchanged; the access log now spends logfield.MaxBytes where it spent maxLogFieldBytes. The sweep the issue asked for found five more call sites of the same shape, all reachable unauthenticated, all now capped: the CSRF 403 (also registered ahead of RequireAuth), the rate limiters' 429 (the per-entrypoint receiver limiter is unauthenticated), RequireAuth's own DEBUG line, the unknown-entrypoint DEBUG line on the receiver, and the failed-login DEBUG lines. DEBUG being off by default is not a bound: an operator turning it on to diagnose a flood must not thereby hand the flood an unbounded write. Every other slog call in the tree was read and judged; the PR body lists all of them, including the ones left alone and why. Two further sites arrived in next with #171 after the first sweep was written and are capped here as well: "login failure limit exceeded" in loginguard.go and "password verification capacity exhausted" in handlers/auth.go, both WARN on the unauthenticated login POST. Neither was ever wide: chi routes that POST on a static pattern, so r.URL.Path is the 12-byte constant /pages/login and each line lands near 120 bytes. They are capped because RecordLoginFailure is exported and takes any *http.Request, so the bound rests on a routing invariant nobody wrote down, and because the same message at handlers/profile.go logs no path at all. No request through the mux can widen either line, so their tests call those two entry points directly with the path a caller on a parameterised route would supply; that is what the caps defend against, and an unasserted cap is one a later edit removes for free. MaxBodySize stays ahead of RequireAuth. An oversize body should be refused before the request buys a cookie decrypt and a session load, and rejecting first is what keeps an unauthenticated flood from choosing how much session work the process does. The ordering and what it costs are now written at the registration, on maxFormBodySize. MaxAccessLogLineBytes is restated as the ceiling on every slog line carrying text an UNAUTHENTICATED client supplies, not just the access log's: each of these lines carries strictly fewer client-supplied fields than the access log does, so none can be wider. That is asserted per line under both handlers rather than argued. The claim is qualified rather than universal because three kinds of writer are outside it, and the README and the constant now name all three: lines carrying an authenticated operator's own input, which are not truncated at all (the webhook name on "webhook created" reaches 600 KB on one line from a 100 KB form field, measured; the SSRF-rejection url and the target_name lines are the same shape) and are left uncapped deliberately, since truncating the operator's own configuration echoed back costs debuggability against no adversary; the log delivery target, which exists to emit the whole event; and GORM's default logger, which prints the interpolated SQL to stdout on a record-not-found and is unbounded on the receiver and login lookups. That last one is a real defect this audit turned up and is filed separately as #178, not fixed here. Tests drive client-chosen text at every site capped here, through both handlers internal/logger can install and through seven fills: plain text, plus the quotation mark, backslash, tab, newline, C0 control and astral non-printable. The C0 control is the one that matters most, costing six bytes on the line against the one it cost to send, and is the case a raw-byte budget breaks on first. The fill is 8 KB everywhere except the two lines past the username lookup, where it is 1 KB because a longer stored username overflows the session cookie and answers 500 before the success line is written. Each case holds the encoded line to the ceiling and asserts the markers at the far end of the input are absent, so a value that merely happened to be short cannot pass. Three of the sites go further and bound the whole flood's output, the total bytes a run of distinct invented values wrote: the 413 rejection, the unknown-entrypoint line and "user not found". The other sites carry the per-line bound only, which is what MaxAccessLogLineBytes states; the README names which sites carry which. internal/logfield gains a test that measures the per-rune charge against what the handlers really emit over roughly 3,000 code points on each, so an undercharged rune fails a test instead of quietly falsifying the ceiling. Verified by mutation: reverting the MaxBodySize cap alone fails 28 subtests with a 16,583-byte line against the 2,560 ceiling; reverting the other five fails 70; reverting either login-throttle WARN cap fails 14, through the direct calls those caps exist for; uncapping either of the two login lines past the username lookup fails both handlers on its own, so those two are independently pinned rather than jointly; budgeting raw bytes instead of encoded ones fails 23 across three packages.
95 lines
2.6 KiB
Go
95 lines
2.6 KiB
Go
package handlers
|
|
|
|
import (
|
|
"html/template"
|
|
"log/slog"
|
|
"net/http"
|
|
|
|
"sneak.berlin/go/webhooker/internal/database"
|
|
)
|
|
|
|
// SetLogForTest replaces the handler's logger, so the handlers_test
|
|
// package can assert on what a log line actually contains rather than
|
|
// on what it is meant to contain.
|
|
func (s *Handlers) SetLogForTest(log *slog.Logger) {
|
|
s.log = log
|
|
}
|
|
|
|
// MaxRenderedBodyBytesForTest exposes the event log's body cap
|
|
// to the handlers_test package.
|
|
const MaxRenderedBodyBytesForTest = maxRenderedBodyBytes
|
|
|
|
// DummyVerificationsForTest reports how many equivalent-cost
|
|
// verifications were charged for usernames that do not exist. It
|
|
// lets a test prove the anti-enumeration path ran without timing
|
|
// anything.
|
|
func (s *Handlers) DummyVerificationsForTest() uint64 {
|
|
return s.dummyVerifications.Load()
|
|
}
|
|
|
|
// TrimPartialRuneForTest exposes trimPartialRune for use in the
|
|
// handlers_test package.
|
|
func TrimPartialRuneForTest(b []byte) []byte {
|
|
return trimPartialRune(b)
|
|
}
|
|
|
|
// LoadEventLogViewsForTest exposes loadEventsWithDeliveries for
|
|
// use in the handlers_test package. Assertions on the projected
|
|
// body need the bytes as loaded: html/template rewrites invalid
|
|
// UTF-8 on the way out, so the rendered page cannot show whether
|
|
// a binary body survived the projection intact.
|
|
func (s *Handlers) LoadEventLogViewsForTest(
|
|
w http.ResponseWriter,
|
|
webhook database.Webhook,
|
|
page int,
|
|
) []EventLogView {
|
|
views, _ := s.loadEventsWithDeliveries(w, webhook, nil, page)
|
|
|
|
return views
|
|
}
|
|
|
|
// AddTemplateForTest registers a template under a page name so that
|
|
// the handlers_test package can drive the render path with a
|
|
// template of its own.
|
|
func (s *Handlers) AddTemplateForTest(
|
|
pageTemplate string,
|
|
tmpl *template.Template,
|
|
) {
|
|
s.templates[pageTemplate] = tmpl
|
|
}
|
|
|
|
// RenderTemplateForTest exposes renderTemplate for use in the
|
|
// handlers_test package.
|
|
func (s *Handlers) RenderTemplateForTest(
|
|
w http.ResponseWriter,
|
|
r *http.Request,
|
|
pageTemplate string,
|
|
data any,
|
|
) {
|
|
s.renderTemplate(w, r, pageTemplate, data)
|
|
}
|
|
|
|
// BuildSlackTargetConfigForTest exposes buildURLTargetConfig
|
|
// with the Slack target parameters for use in the
|
|
// handlers_test package.
|
|
func (s *Handlers) BuildSlackTargetConfigForTest(
|
|
w http.ResponseWriter,
|
|
r *http.Request,
|
|
targetURL string,
|
|
) (string, error) {
|
|
return s.buildURLTargetConfig(
|
|
w, r, targetURL, "webhookUrl",
|
|
"Webhook URL is required for Slack targets",
|
|
)
|
|
}
|
|
|
|
// BuildDatabaseTargetConfigForTest exposes
|
|
// buildDatabaseTargetConfig for use in the handlers_test
|
|
// package.
|
|
func (s *Handlers) BuildDatabaseTargetConfigForTest(
|
|
w http.ResponseWriter,
|
|
expiry string,
|
|
) (string, error) {
|
|
return s.buildDatabaseTargetConfig(w, expiry)
|
|
}
|