diff --git a/internal/ingress/annotations/auth/main.go b/internal/ingress/annotations/auth/main.go index 7c7fde7b8..79e3ce5d3 100644 --- a/internal/ingress/annotations/auth/main.go +++ b/internal/ingress/annotations/auth/main.go @@ -19,6 +19,7 @@ package auth import ( "fmt" "os" + "path/filepath" "regexp" "strings" @@ -203,16 +204,24 @@ func (a auth) Parse(ing *networking.Ingress) (interface{}, error) { return nil, err } - passFilename := fmt.Sprintf("%v/%v-%v-%v.passwd", a.authDirectory, ing.GetNamespace(), ing.UID, secret.UID) + passFileName := fmt.Sprintf("%v-%v-%v.passwd", ing.GetNamespace(), ing.UID, secret.UID) + passFilePath := filepath.Join(a.authDirectory, passFileName) + + // Ensure password file name does not contain any path traversal characters. + if a.authDirectory != filepath.Dir(passFilePath) || passFileName != filepath.Base(passFilePath) { + return nil, ing_errors.LocationDeniedError{ + Reason: fmt.Errorf("invalid password file name: %s", passFileName), + } + } switch secretType { case fileAuth: - err = dumpSecretAuthFile(passFilename, secret) + err = dumpSecretAuthFile(passFilePath, secret) if err != nil { return nil, err } case mapAuth: - err = dumpSecretAuthMap(passFilename, secret) + err = dumpSecretAuthMap(passFilePath, secret) if err != nil { return nil, err } @@ -225,9 +234,9 @@ func (a auth) Parse(ing *networking.Ingress) (interface{}, error) { return &Config{ Type: at, Realm: realm, - File: passFilename, + File: passFilePath, Secured: true, - FileSHA: file.SHA1(passFilename), + FileSHA: file.SHA1(passFilePath), Secret: name, SecretType: secretType, }, nil diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index aa8f4c4b9..012f465bf 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -420,11 +420,15 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error { return err } + /* Deactivated to mitigate CVE-2025-1974 + // TODO: Implement sandboxing so this test can be done safely err = n.testTemplate(content) if err != nil { n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name) return err } + */ + n.metricCollector.IncCheckCount(ing.ObjectMeta.Namespace, ing.Name) endCheck := time.Now().UnixNano() / 1000000 n.metricCollector.SetAdmissionMetrics( diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index 9d3fea470..3b7a3c4eb 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -250,6 +250,8 @@ func TestCheckIngress(t *testing.T) { } }) + /* Deactivated to mitigate CVE-2025-1974 + // TODO: Implement sandboxing so this test can be done safely t.Run("When nginx test returns an error", func(t *testing.T) { nginx.command = testNginxTestCommand{ t: t, @@ -261,6 +263,7 @@ func TestCheckIngress(t *testing.T) { t.Errorf("with a new ingress with an error, an error should be returned") } }) + */ t.Run("When the default annotation prefix is used despite an override", func(t *testing.T) { defer func() { diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index d2c8a05a9..f6816d70a 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -1627,11 +1627,11 @@ func buildMirrorLocations(locs []*ingress.Location) string { mapped.Insert(loc.Mirror.Source) buffer.WriteString(fmt.Sprintf(`location = %v { internal; -proxy_set_header Host "%v"; -proxy_pass "%v"; +proxy_set_header Host %v; +proxy_pass %v; } -`, loc.Mirror.Source, loc.Mirror.Host, loc.Mirror.Target)) +`, strconv.Quote(loc.Mirror.Source), strconv.Quote(loc.Mirror.Host), strconv.Quote(loc.Mirror.Target))) } return buffer.String() diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index f32860dc2..1f17952a1 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -875,7 +875,7 @@ stream { {{ if not ( empty $server.CertificateAuth.MatchCN ) }} {{ if gt (len $server.CertificateAuth.MatchCN) 0 }} - if ( $ssl_client_s_dn !~ {{ $server.CertificateAuth.MatchCN }} ) { + if ( $ssl_client_s_dn !~ {{ $server.CertificateAuth.MatchCN | quote }} ) { return 403 "client certificate unauthorized"; } {{ end }} @@ -1077,7 +1077,7 @@ stream { set $target {{ changeHostPort $externalAuth.URL $authUpstreamName }}; {{ else }} proxy_http_version {{ $location.Proxy.ProxyHTTPVersion }}; - set $target {{ $externalAuth.URL }}; + set $target {{ $externalAuth.URL | quote }}; {{ end }} proxy_pass $target; } @@ -1115,7 +1115,7 @@ stream { {{ buildOpentelemetryForLocation $all.Cfg.EnableOpentelemetry $all.Cfg.OpentelemetryTrustIncomingSpan $location }} {{ if $location.Mirror.Source }} - mirror {{ $location.Mirror.Source }}; + mirror {{ $location.Mirror.Source | quote }}; mirror_request_body {{ $location.Mirror.RequestBody }}; {{ end }} diff --git a/test/e2e/admission/admission.go b/test/e2e/admission/admission.go index 873e6719d..2c94a5893 100644 --- a/test/e2e/admission/admission.go +++ b/test/e2e/admission/admission.go @@ -26,7 +26,6 @@ import ( "github.com/onsi/ginkgo/v2" "github.com/stretchr/testify/assert" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/ingress-nginx/test/e2e/framework" @@ -99,6 +98,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller", assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid path should return an error") }) + /* Deactivated to mitigate CVE-2025-1974 + // TODO: Implement sandboxing so this test can be done safely ginkgo.It("should return an error if there is an error validating the ingress definition", func() { disableSnippet := f.AllowSnippetConfiguration() defer disableSnippet() @@ -112,6 +113,7 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller", _, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{}) assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error") }) + */ ginkgo.It("should return an error if there is an invalid value in some annotation", func() { host := admissionTestHost @@ -207,6 +209,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller", Status(http.StatusOK) }) + /* Deactivated to mitigate CVE-2025-1974 + // TODO: Implement sandboxing so this test can be done safely ginkgo.It("should return an error if the Ingress V1 definition contains invalid annotations", func() { disableSnippet := f.AllowSnippetConfiguration() defer disableSnippet() @@ -220,6 +224,7 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller", assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error") } }) + */ ginkgo.It("should not return an error for an invalid Ingress when it has unknown class", func() { disableSnippet := f.AllowSnippetConfiguration() diff --git a/test/e2e/annotations/mirror.go b/test/e2e/annotations/mirror.go index 787cbfa3b..a398a21e3 100644 --- a/test/e2e/annotations/mirror.go +++ b/test/e2e/annotations/mirror.go @@ -43,7 +43,7 @@ var _ = framework.DescribeAnnotation("mirror-*", func() { f.WaitForNginxServer(host, func(server string) bool { - return strings.Contains(server, fmt.Sprintf("mirror /_mirror-%v;", ing.UID)) && + return strings.Contains(server, fmt.Sprintf("mirror \"/_mirror-%v\";", ing.UID)) && strings.Contains(server, "mirror_request_body on;") }) }) @@ -58,7 +58,7 @@ var _ = framework.DescribeAnnotation("mirror-*", func() { f.WaitForNginxServer(host, func(server string) bool { - return strings.Contains(server, fmt.Sprintf("mirror /_mirror-%v;", ing.UID)) && + return strings.Contains(server, fmt.Sprintf("mirror \"/_mirror-%v\";", ing.UID)) && strings.Contains(server, "mirror_request_body on;") && strings.Contains(server, `proxy_pass "https://test.env.com/$request_uri";`) }) @@ -75,7 +75,7 @@ var _ = framework.DescribeAnnotation("mirror-*", func() { f.WaitForNginxServer(host, func(server string) bool { - return strings.Contains(server, fmt.Sprintf("mirror /_mirror-%v;", ing.UID)) && + return strings.Contains(server, fmt.Sprintf("mirror \"/_mirror-%v\";", ing.UID)) && strings.Contains(server, "mirror_request_body off;") }) })