Merge remote-tracking branch 'blotus/no-panic-on-non-selectable-col' …

…into perftest

experimental/plugins/plugintypes/transaction.go

@@ -100,20 +100,20 @@ type TransactionVariables interface {
 	RequestHeaders() collection.Map
 	ResponseHeaders() collection.Map
 	MultipartName() collection.Map
-	MatchedVarsNames() collection.Collection
+	MatchedVarsNames() collection.Keyed
 	MultipartFilename() collection.Map
 	MatchedVars() collection.Map
 	FilesSizes() collection.Map
 	FilesNames() collection.Map
 	FilesTmpContent() collection.Map
-	ResponseHeadersNames() collection.Collection
-	RequestHeadersNames() collection.Collection
-	RequestCookiesNames() collection.Collection
+	ResponseHeadersNames() collection.Keyed
+	RequestHeadersNames() collection.Keyed
+	RequestCookiesNames() collection.Keyed
 	XML() collection.Map
 	RequestXML() collection.Map
 	ResponseXML() collection.Map
-	ArgsNames() collection.Collection
-	ArgsGetNames() collection.Collection
-	ArgsPostNames() collection.Collection
+	ArgsNames() collection.Keyed
+	ArgsGetNames() collection.Keyed
+	ArgsPostNames() collection.Keyed
 	MultipartStrictError() collection.Single
 }

http/middleware_test.go

@@ -332,6 +332,12 @@ func TestHttpServer(t *testing.T) {
 			expectedStatus:          403,
 			expectedRespHeadersKeys: expectedBlockingHeaders,
 		},
+		"deny based on number of post arguments matching a name": {
+			reqURI:                  "/hello?foobar=1&foobar=2",
+			expectedProto:           "HTTP/1.1",
+			expectedStatus:          403,
+			expectedRespHeadersKeys: expectedBlockingHeaders,
+		},
 	}
 
 	logger := debuglog.Default().
@@ -358,6 +364,7 @@ func TestHttpServer(t *testing.T) {
 	SecRule RESPONSE_HEADERS:Foo "@pm bar" "id:199,phase:3,deny,t:lowercase,deny, status:401,msg:'Invalid response header',log,auditlog"
 	SecRule RESPONSE_BODY "@contains password" "id:200, phase:4,deny, status:403,msg:'Invalid response body',log,auditlog"
 	SecRule REQUEST_URI "/allow_me" "id:9,phase:1,allow,msg:'ALLOWED'"
+	SecRule &ARGS_GET_NAMES:foobar "@eq 2" "id:11,phase:1,deny, status:403,msg:'Invalid foobar',log,auditlog"
 `).WithErrorCallback(errLogger(t)).WithDebugLogger(logger)
 			if l := tCase.reqBodyLimit; l > 0 {
 				conf = conf.WithRequestBodyAccess().WithRequestBodyLimit(l).WithRequestBodyInMemoryLimit(l)

internal/collections/named.go

@@ -80,7 +80,7 @@ func (c *NamedCollection) Reset() {
 	c.Map.Reset()
 }
 
-func (c *NamedCollection) Names(rv variables.RuleVariable) collection.Collection {
+func (c *NamedCollection) Names(rv variables.RuleVariable) collection.Keyed {
 	return &NamedCollectionNames{
 		variable:   rv,
 		collection: c,
@@ -101,11 +101,43 @@ type NamedCollectionNames struct {
 }
 
 func (c *NamedCollectionNames) FindRegex(key *regexp.Regexp) []types.MatchData {
-	panic("selection operator not supported")
+	var res []types.MatchData
+
+	for k, data := range c.collection.Map.data {
+		if !key.MatchString(k) {
+			continue
+		}
+		for _, d := range data {
+			res = append(res, &corazarules.MatchData{
+				Variable_: c.variable,
+				Key_:      d.key,
+				Value_:    d.key,
+			})
+		}
+	}
+	return res
 }
 
 func (c *NamedCollectionNames) FindString(key string) []types.MatchData {
-	panic("selection operator not supported")
+	var res []types.MatchData
+
+	for k, data := range c.collection.Map.data {
+		if k != key {
+			continue
+		}
+		for _, d := range data {
+			res = append(res, &corazarules.MatchData{
+				Variable_: c.variable,
+				Key_:      d.key,
+				Value_:    d.key,
+			})
+		}
+	}
+	return res
+}
+
+func (c *NamedCollectionNames) Get(key string) []string {
+	return c.collection.Map.Get(key)
 }
 
 func (c *NamedCollectionNames) FindAll() []types.MatchData {

internal/collections/named_test.go

@@ -87,3 +87,39 @@ func TestNamedCollection(t *testing.T) {
 	}
 
 }
+
+func TestNames(t *testing.T) {
+	c := NewNamedCollection(variables.ArgsPost)
+	if c.Name() != "ARGS_POST" {
+		t.Error("Error getting name")
+	}
+
+	c.SetIndex("key", 1, "value")
+	c.Set("key2", []string{"value2", "value3"})
+
+	names := c.Names(variables.ArgsPostNames)
+
+	r := names.FindString("key2")
+
+	if len(r) != 2 {
+		t.Errorf("Error finding string, got %d instead of 2", len(r))
+	}
+
+	r = names.FindString("nonexistent")
+
+	if len(r) != 0 {
+		t.Errorf("Error finding nonexistent, got %d instead of 0", len(r))
+	}
+
+	r = names.FindRegex(regexp.MustCompile("key.*"))
+
+	if len(r) != 3 {
+		t.Errorf("Error finding regex, got %d instead of 3", len(r))
+	}
+
+	r = names.FindRegex(regexp.MustCompile("nonexistent"))
+
+	if len(r) != 0 {
+		t.Errorf("Error finding nonexistent regex, got %d instead of 0", len(r))
+	}
+}

internal/corazawaf/transaction.go

@@ -575,13 +575,15 @@ func (tx *Transaction) GetField(rv ruleVariableParams) []experimentalTypes.Match
 		if m, ok := col.(collection.Keyed); ok {
 			matches = m.FindRegex(rv.KeyRx)
 		} else {
-			panic("attempted to use regex with non-selectable collection: " + rv.Variable.Name())
+			// This should probably never happen, selectability is checked at parsing time
+			tx.debugLogger.Error().Str("collection", rv.Variable.Name()).Msg("attempted to use regex with non-selectable collection")
 		}
 	case rv.KeyStr != "":
 		if m, ok := col.(collection.Keyed); ok {
 			matches = m.FindString(rv.KeyStr)
 		} else {
-			panic("attempted to use string with non-selectable collection: " + rv.Variable.Name())
+			// This should probably never happen, selectability is checked at parsing time
+			tx.debugLogger.Error().Str("collection", rv.Variable.Name()).Msg("attempted to use string with non-selectable collection")
 		}
 	default:
 		matches = col.FindAll()
@@ -1597,11 +1599,11 @@ type TransactionVariables struct {
 	args                     *collections.ConcatKeyed
 	argsCombinedSize         *collections.SizeCollection
 	argsGet                  *collections.NamedCollection
-	argsGetNames             collection.Collection
-	argsNames                *collections.ConcatCollection
+	argsGetNames             collection.Keyed
+	argsNames                *collections.ConcatKeyed
 	argsPath                 *collections.NamedCollection
 	argsPost                 *collections.NamedCollection
-	argsPostNames            collection.Collection
+	argsPostNames            collection.Keyed
 	duration                 *collections.Single
 	env                      *collections.Map
 	files                    *collections.Map
@@ -1617,7 +1619,7 @@ type TransactionVariables struct {
 	matchedVar               *collections.Single
 	matchedVarName           *collections.Single
 	matchedVars              *collections.NamedCollection
-	matchedVarsNames         collection.Collection
+	matchedVarsNames         collection.Keyed
 	multipartDataAfter       *collections.Single
 	multipartFilename        *collections.Map
 	multipartName            *collections.Map
@@ -1637,10 +1639,10 @@ type TransactionVariables struct {
 	requestBody              *collections.Single
 	requestBodyLength        *collections.Single
 	requestCookies           *collections.NamedCollection
-	requestCookiesNames      collection.Collection
+	requestCookiesNames      collection.Keyed
 	requestFilename          *collections.Single
 	requestHeaders           *collections.NamedCollection
-	requestHeadersNames      collection.Collection
+	requestHeadersNames      collection.Keyed
 	requestLine              *collections.Single
 	requestMethod            *collections.Single
 	requestProtocol          *collections.Single
@@ -1651,7 +1653,7 @@ type TransactionVariables struct {
 	responseContentLength    *collections.Single
 	responseContentType      *collections.Single
 	responseHeaders          *collections.NamedCollection
-	responseHeadersNames     collection.Collection
+	responseHeadersNames     collection.Keyed
 	responseProtocol         *collections.Single
 	responseStatus           *collections.Single
 	responseXML              *collections.Map
@@ -1765,7 +1767,7 @@ func NewTransactionVariables() *TransactionVariables {
 		v.argsPost,
 		v.argsPath,
 	)
-	v.argsNames = collections.NewConcatCollection(
+	v.argsNames = collections.NewConcatKeyed(
 		variables.ArgsNames,
 		v.argsGetNames,
 		v.argsPostNames,
@@ -1991,7 +1993,7 @@ func (v *TransactionVariables) MultipartName() collection.Map {
 	return v.multipartName
 }
 
-func (v *TransactionVariables) MatchedVarsNames() collection.Collection {
+func (v *TransactionVariables) MatchedVarsNames() collection.Keyed {
 	return v.matchedVarsNames
 }
 
@@ -2015,19 +2017,19 @@ func (v *TransactionVariables) FilesTmpContent() collection.Map {
 	return v.filesTmpContent
 }
 
-func (v *TransactionVariables) ResponseHeadersNames() collection.Collection {
+func (v *TransactionVariables) ResponseHeadersNames() collection.Keyed {
 	return v.responseHeadersNames
 }
 
 func (v *TransactionVariables) ResponseArgs() collection.Map {
 	return v.responseArgs
 }
 
-func (v *TransactionVariables) RequestHeadersNames() collection.Collection {
+func (v *TransactionVariables) RequestHeadersNames() collection.Keyed {
 	return v.requestHeadersNames
 }
 
-func (v *TransactionVariables) RequestCookiesNames() collection.Collection {
+func (v *TransactionVariables) RequestCookiesNames() collection.Keyed {
 	return v.requestCookiesNames
 }
 
@@ -2047,15 +2049,15 @@ func (v *TransactionVariables) ResponseBodyProcessor() collection.Single {
 	return v.resBodyProcessor
 }
 
-func (v *TransactionVariables) ArgsNames() collection.Collection {
+func (v *TransactionVariables) ArgsNames() collection.Keyed {
 	return v.argsNames
 }
 
-func (v *TransactionVariables) ArgsGetNames() collection.Collection {
+func (v *TransactionVariables) ArgsGetNames() collection.Keyed {
 	return v.argsGetNames
 }
 
-func (v *TransactionVariables) ArgsPostNames() collection.Collection {
+func (v *TransactionVariables) ArgsPostNames() collection.Keyed {
 	return v.argsPostNames
 }
 

internal/seclang/rule_parser.go

@@ -70,6 +70,9 @@ func (rp *RuleParser) ParseVariables(vars string) error {
 			if err != nil {
 				return err
 			}
+			if curr == 1 && !v.CanBeSelected() {
+				return fmt.Errorf("attempting to select a value inside a non-selectable collection: %s", string(curVar))
+			}
 			// fmt.Printf("(PREVIOUS %s) %s:%s (%t %t)\n", vars, curvar, curkey, iscount, isnegation)
 			if isquoted {
 				// if it is quoted we remove the last quote

internal/seclang/rule_parser_test.go

@@ -303,6 +303,17 @@ func TestParseRule(t *testing.T) {
 	}
 }
 
+func TestNonSelectableCollection(t *testing.T) {
+	waf := corazawaf.NewWAF()
+	p := NewParser(waf)
+	err := p.FromString(`
+	SecRule REQUEST_URI:foo "bar" "id:1,phase:1"
+	`)
+	if err == nil {
+		t.Error("expected error")
+	}
+}
+
 func BenchmarkParseActions(b *testing.B) {
 	actionsToBeParsed := "id:980170,phase:5,pass,t:none,noauditlog,msg:'Anomaly Scores:Inbound Scores - Outbound Scores',tag:test"
 	for i := 0; i < b.N; i++ {

internal/variables/generator/main.go

@@ -23,8 +23,9 @@ import (
 var variablesMapTmpl string
 
 type VariablesMap struct {
-	Key   string
-	Value string
+	Key           string
+	Value         string
+	CanBeSelected bool
 }
 
 func main() {
@@ -74,9 +75,19 @@ func main() {
 						value = "FILES_TMPNAMES"
 					}
 
+					canBeSelected := false
+					if v.Comment != nil {
+						for _, c := range v.Comment.List {
+							if strings.Contains(c.Text, "CanBeSelected") {
+								canBeSelected = true
+							}
+						}
+					}
+
 					directives = append(directives, VariablesMap{
-						Key:   name.String(),
-						Value: value,
+						Key:           name.String(),
+						Value:         value,
+						CanBeSelected: canBeSelected,
 					})
 				}
 			}

internal/variables/generator/variablesmap.go.tmpl

@@ -22,6 +22,20 @@ func (v RuleVariable) Name() string {
     }
 }
 
+// CanBeSelected returns true if the variable supports selection (ie, `:foobar`)
+func (v RuleVariable) CanBeSelected() bool {
+    switch v {
+    {{- range . }}
+    {{- if .CanBeSelected }}
+    case {{ .Key }}:
+        return true
+    {{- end }}
+    {{- end }}
+    default:
+        return false
+    }
+}
+
 var rulemapRev = map[string]RuleVariable{
     {{range .}}"{{ .Value }}": {{ .Key }},
     {{end}}