From e77e1427679fc3b18111cfc565559dc661ab789f Mon Sep 17 00:00:00 2001
From: Puerco <puerco@users.noreply.github.com>
Date: Wed, 10 Jul 2024 16:23:14 -0600
Subject: [PATCH] Artifact tag matcher: Curb complexity when parsing regexps
 from user input (#3836)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Mitigate ReDos in artifact tag matcher

This commit adds a limit to the lengrh of regular expressions we take
from user input to set a length limit and curve the complexity Minder
handles when compiling them.

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@stacklok.com>

* Add test for artifact.BuildFilter

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@stacklok.com>

---------

Signed-off-by: Adolfo García Veytia (Puerco) <puerco@stacklok.com>
---
 internal/providers/artifact/versionsfilter.go |  3 +
 .../providers/artifact/versionsfilter_test.go | 75 +++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/internal/providers/artifact/versionsfilter.go b/internal/providers/artifact/versionsfilter.go
index bfb948705c..64d02c537d 100644
--- a/internal/providers/artifact/versionsfilter.go
+++ b/internal/providers/artifact/versionsfilter.go
@@ -47,6 +47,9 @@ func BuildFilter(tags []string, tagRegex string) (*filter, error) {
 
 	// no tags specified, but a regex was, compile it
 	if tagRegex != "" {
+		if len(tagRegex) > 512 {
+			return nil, fmt.Errorf("tag regular expressions are limited to 512 characters")
+		}
 		re, err := regexp.Compile(tagRegex)
 		if err != nil {
 			return nil, fmt.Errorf("error compiling tag regex: %w", err)
diff --git a/internal/providers/artifact/versionsfilter_test.go b/internal/providers/artifact/versionsfilter_test.go
index 678204b3eb..ea258468b0 100644
--- a/internal/providers/artifact/versionsfilter_test.go
+++ b/internal/providers/artifact/versionsfilter_test.go
@@ -16,14 +16,89 @@
 package artifact
 
 import (
+	"regexp"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	provifv1 "github.com/stacklok/minder/pkg/providers/v1"
 )
 
+func TestBuildFilter(t *testing.T) {
+	t.Parallel()
+	simpleRegex := `^simpleregex$`
+	compiledSimpleRegex := regexp.MustCompile(simpleRegex)
+	for _, tc := range []struct {
+		name     string
+		tags     []string
+		regex    string
+		expected *filter
+		mustErr  bool
+	}{
+		{
+			name:    "tags",
+			tags:    []string{"hello", "bye"},
+			mustErr: false,
+			expected: &filter{
+				tagMatcher:      &tagListMatcher{tags: []string{"hello", "bye"}},
+				retentionPeriod: time.Time{},
+			},
+		},
+		{
+			name:    "empty-tag",
+			tags:    []string{"hello", ""},
+			mustErr: true,
+		},
+		{
+			name:    "regex",
+			tags:    []string{},
+			regex:   simpleRegex,
+			mustErr: false,
+			expected: &filter{
+				tagMatcher: &tagRegexMatcher{
+					re: compiledSimpleRegex,
+				},
+			},
+		},
+		{
+			name:    "invalidregexp",
+			tags:    []string{},
+			regex:   `$(invalid^`,
+			mustErr: true,
+		},
+		{
+			name:    "valid-long-regexp",
+			tags:    []string{},
+			regex:   `^` + strings.Repeat("A", 1000) + `$`,
+			mustErr: true,
+		},
+		{
+			name:    "no-tags",
+			tags:    []string{},
+			mustErr: false,
+			expected: &filter{
+				tagMatcher: &tagAllMatcher{},
+			},
+		},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			f, err := BuildFilter(tc.tags, tc.regex)
+			if tc.mustErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			require.NotNil(t, f.tagMatcher)
+			require.Equal(t, tc.expected.tagMatcher, f.tagMatcher)
+		})
+	}
+}
+
 func Test_filter_IsSkippable(t *testing.T) {
 	t.Parallel()