diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..7a104ca --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,8 @@ +## 2024-10-18 - [Security Discovery Journal] +**Vulnerability:** N/A yet, exploring +**Learning:** N/A yet +**Prevention:** N/A yet +## 2024-10-18 - CRLF Injection via Environment Variable Expansion +**Vulnerability:** HTTP Response splitting/CRLF injection is possible when an environment variable containing `\r\n` is used inside a response header via `${VAR}` expansion. The validation of `Spec.Headers` happens BEFORE environment variables are expanded, allowing the rendered response to have headers with CRLF, causing `http.ResponseWriter` to split the response. +**Learning:** Checking inputs early is good, but when inputs are dynamically evaluated (e.g. `EnvPolicy.Expand`), the expanded form must either be validated again or the expansion logic must sanitize its output. The system validated `s.Headers` in `Set()` but didn't catch CRLF introduced by `${VAR}` at `Render()` time. +**Prevention:** Either validate the expanded headers in `Render()` and strip/reject invalid characters, or sanitize the output of `EnvPolicy.Expand` when it's used in header values. We will sanitize the header value directly during expansion in `Render()` or `EnvPolicy.Expand` by replacing `\n` and `\r` with spaces, as Go's `http.ResponseWriter` handles CRLF in headers very poorly depending on how it's written. diff --git a/internal/responder/responder.go b/internal/responder/responder.go index 6eaa674..6b47600 100644 --- a/internal/responder/responder.go +++ b/internal/responder/responder.go @@ -167,12 +167,27 @@ func (r *Responder) Set(s Spec) error { return nil } +func sanitizeHeaderValue(v string) string { + if validHeaderValue(v) { + return v + } + var b strings.Builder + b.Grow(len(v)) + for i := 0; i < len(v); i++ { + c := v[i] + if c == '\t' || (c >= 0x20 && c != 0x7f) { + b.WriteByte(c) + } + } + return b.String() +} + // Render returns the reply to send now, with placeholders expanded. func (r *Responder) Render() events.Response { s := r.Get() headers := make([]events.Header, len(s.Headers)) for i, h := range s.Headers { - headers[i] = events.Header{Name: h.Name, Value: r.env.Expand(h.Value)} + headers[i] = events.Header{Name: h.Name, Value: sanitizeHeaderValue(r.env.Expand(h.Value))} } return events.Response{ Status: s.Status, diff --git a/internal/responder/responder_test.go b/internal/responder/responder_test.go index d2d462a..8045f51 100644 --- a/internal/responder/responder_test.go +++ b/internal/responder/responder_test.go @@ -72,6 +72,24 @@ func TestResponderSetValidatesAndRenders(t *testing.T) { } } +func TestRenderSanitizesCRLFInHeaders(t *testing.T) { + env := map[string]string{"PORTBELL_INJECT": "a\r\nb\tc\n\x00d"} + p := EnvPolicy{Prefix: "PORTBELL_", Lookup: func(k string) (string, bool) { + v, ok := env[k] + return v, ok + }} + r := New(Spec{ + Status: 200, + Headers: []events.Header{ + {Name: "X-Inject", Value: "${PORTBELL_INJECT}"}, + }, + }, p) + got := r.Render() + if len(got.Headers) != 1 || got.Headers[0].Value != "ab\tcd" { + t.Errorf("unexpected header value: %q", got.Headers[0].Value) + } +} + func TestParseHTTP(t *testing.T) { in := "HTTP/1.1 201 Created\r\nContent-Type: application/json\r\nX-A: b \r\n\r\n{\"ok\": true}\nline2" spec, err := ParseHTTP(strings.NewReader(in)) diff --git a/internal/server/server.go b/internal/server/server.go index 5883929..784fee7 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -95,7 +95,8 @@ func (s *Server) Serve(ctx context.Context) error { if err := srv.Shutdown(shutdownCtx); err != nil { _ = srv.Close() } - <-errc + // Wait for the server to finish + <-errc //nolint:errcheck return nil } }