From bab16c75ccdaba882c8fcb4c1813eae365a23b2c Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Fri, 1 May 2026 12:17:42 +1000 Subject: [PATCH] fix(elbv2): validate HAProxy inputs (#181) --- spinifex/handlers/elbv2/haproxy.go | 6 ++ spinifex/handlers/elbv2/service_impl.go | 33 +++++++ spinifex/handlers/elbv2/service_impl_test.go | 90 ++++++++++++++++++++ 3 files changed, 129 insertions(+) diff --git a/spinifex/handlers/elbv2/haproxy.go b/spinifex/handlers/elbv2/haproxy.go index 5c3cb9b4f..af4f11550 100644 --- a/spinifex/handlers/elbv2/haproxy.go +++ b/spinifex/handlers/elbv2/haproxy.go @@ -13,6 +13,12 @@ import ( "text/template" ) +// Every interpolated string field in the templates below MUST be regex-validated +// at its input boundary. A newline or HAProxy keyword in any unvalidated field +// would inject arbitrary directives into the rendered config. The current +// surface is HealthCheckPath and HealthCheckMatcher (validated in +// CreateTargetGroup); all other interpolated identifiers go through +// sanitizeName or are integers. const haproxyConfigTemplate = `# Auto-generated by spinifex ELBv2 — do not edit global log stdout format short daemon diff --git a/spinifex/handlers/elbv2/service_impl.go b/spinifex/handlers/elbv2/service_impl.go index ecc35664c..977e650bf 100644 --- a/spinifex/handlers/elbv2/service_impl.go +++ b/spinifex/handlers/elbv2/service_impl.go @@ -7,6 +7,7 @@ import ( "fmt" "log/slog" "maps" + "regexp" "slices" "strings" "sync" @@ -35,8 +36,34 @@ const ( // change) writes the full LoadBalancerRecord back to KV. State transitions // always persist immediately. heartbeatPersistInterval = 60 * time.Second + + // Health check fields are interpolated verbatim into the HAProxy template + // (see haproxy.go); restrict them to characters that cannot terminate a + // directive or introduce a new one. Length caps bound abuse and match the + // surface AWS exposes for these fields. + maxHealthCheckPathLen = 1024 + maxHealthCheckMatcherLen = 64 ) +var ( + healthCheckPathRegex = regexp.MustCompile(`^[A-Za-z0-9._~/?#%&=+-]+$`) + healthCheckMatcherRegex = regexp.MustCompile(`^[0-9,-]+$`) +) + +func validateHealthCheckPath(p string) error { + if len(p) == 0 || len(p) > maxHealthCheckPathLen || !healthCheckPathRegex.MatchString(p) { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + return nil +} + +func validateHealthCheckMatcher(m string) error { + if len(m) == 0 || len(m) > maxHealthCheckMatcherLen || !healthCheckMatcherRegex.MatchString(m) { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + return nil +} + // lbVMUserData generates cloud-config user data for a load balancer VM. // Uses write_files to populate /etc/conf.d/lb-agent with the lb-id, gateway // URL, and system credentials for SigV4 auth. The CA cert is already injected @@ -938,6 +965,9 @@ func (s *ELBv2ServiceImpl) CreateTargetGroup(input *elbv2.CreateTargetGroupInput hc.Port = *input.HealthCheckPort } if input.HealthCheckPath != nil { + if err := validateHealthCheckPath(*input.HealthCheckPath); err != nil { + return nil, err + } hc.Path = *input.HealthCheckPath } if input.HealthCheckIntervalSeconds != nil { @@ -953,6 +983,9 @@ func (s *ELBv2ServiceImpl) CreateTargetGroup(input *elbv2.CreateTargetGroupInput hc.UnhealthyThreshold = *input.UnhealthyThresholdCount } if input.Matcher != nil && input.Matcher.HttpCode != nil { + if err := validateHealthCheckMatcher(*input.Matcher.HttpCode); err != nil { + return nil, err + } hc.Matcher = *input.Matcher.HttpCode } diff --git a/spinifex/handlers/elbv2/service_impl_test.go b/spinifex/handlers/elbv2/service_impl_test.go index 0d5f1be0a..874e87b8c 100644 --- a/spinifex/handlers/elbv2/service_impl_test.go +++ b/spinifex/handlers/elbv2/service_impl_test.go @@ -3,6 +3,7 @@ package handlers_elbv2 import ( "errors" "sort" + "strings" "sync" "testing" "time" @@ -416,6 +417,95 @@ func TestCreateTargetGroup_TCPWithCustomHealthCheck(t *testing.T) { assert.Equal(t, int64(15), *tg.HealthCheckIntervalSeconds) } +func TestCreateTargetGroup_RejectsHealthCheckPathInjection(t *testing.T) { + svc := setupTestService(t) + + cases := map[string]string{ + "newline_haproxy_directive": "/x\nbind 0.0.0.0:9999", + "trailing_directive": "/x\n use_backend other", + "crlf": "/x\r\nbind 0.0.0.0:9999", + "cr_only": "/x\rbind", + "tab": "/x\tuse_backend other", + "semicolon_drop": "/x; drop", + "jndi": "${jndi:ldap://x}", + "space": "/path with space", + "leading_space": " /healthz", + "trailing_space": "/healthz ", + "whitespace_only": " ", + "multibyte_utf8": "/healthz・", + "empty": "", + "too_long": strings.Repeat("a", maxHealthCheckPathLen+1), + "control_byte": "/x\x00", + } + + for name, payload := range cases { + t.Run(name, func(t *testing.T) { + _, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("inj-" + name), + HealthCheckPath: aws.String(payload), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "InvalidParameterValue") + }) + } +} + +func TestCreateTargetGroup_RejectsMatcherInjection(t *testing.T) { + svc := setupTestService(t) + + cases := map[string]string{ + "newline_use_backend": "200\nuse_backend other", + "crlf": "200\r\nuse_backend other", + "tab": "200\t201", + "alpha": "abc", + "empty": "", + "whitespace_only": " ", + "multibyte_digits": "２００", + "too_long": strings.Repeat("9", maxHealthCheckMatcherLen+1), + "with_space": "200 ,201", + } + + for name, payload := range cases { + t.Run(name, func(t *testing.T) { + _, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("matcher-" + name), + Matcher: &elbv2.Matcher{HttpCode: aws.String(payload)}, + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "InvalidParameterValue") + }) + } +} + +func TestCreateTargetGroup_AcceptsValidHealthCheckInputs(t *testing.T) { + svc := setupTestService(t) + + out, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("ok-tg"), + HealthCheckPath: aws.String("/healthz?probe=lb#frag"), + Matcher: &elbv2.Matcher{HttpCode: aws.String("200-299,301")}, + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, "/healthz?probe=lb#frag", *out.TargetGroups[0].HealthCheckPath) + assert.Equal(t, "200-299,301", *out.TargetGroups[0].Matcher.HttpCode) +} + +func TestCreateTargetGroup_AcceptsHealthCheckBoundaryLengths(t *testing.T) { + svc := setupTestService(t) + + maxPath := "/" + strings.Repeat("a", maxHealthCheckPathLen-1) + maxMatcher := "2" + strings.Repeat("0", maxHealthCheckMatcherLen-1) + + out, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ + Name: aws.String("boundary-tg"), + HealthCheckPath: aws.String(maxPath), + Matcher: &elbv2.Matcher{HttpCode: aws.String(maxMatcher)}, + }, testAccountID) + require.NoError(t, err) + assert.Equal(t, maxPath, *out.TargetGroups[0].HealthCheckPath) + assert.Equal(t, maxMatcher, *out.TargetGroups[0].Matcher.HttpCode) +} + func TestCreateTargetGroup_DuplicateName(t *testing.T) { svc := setupTestService(t)