🛡️ Sentinel: [HIGH] Fix CRLF Injection via Environment Variable Expansion

.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.

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,

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))

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
 	}
 }