dynamic rules: @fliter capturing any pattern not only variable (#1249)

docs/dynamic_rules.md

@@ -492,7 +492,7 @@ eval(${"var"});
 
 ##### `@filter`
 
-The `@filter` restriction allows you to restrict the rule by name of matched variable.
+The `@filter` restriction allows you to restrict the rule by name of matched variable or literal.
 
 Thus, the rule will be applied only if there is a matched variable that matches passed regexp.
 
@@ -510,8 +510,6 @@ function forbiddenIdUsage() {
 
 This rule will match usage if `$id` variable.
 
-Also, originally designed to apply to variables by matching their names, the functionality has been extended so that if a string literal is captured under the given name, the regular expression is applied to its literal value.
-
 When a string literal is captured, the regular expression is applied to the literal’s value.
 
 For example:
@@ -528,6 +526,29 @@ function insecureUrl() {
 
 In this case, the rule will match because the captured string literal "http://example.com" matches the regular expression ^http://.
 
+Let see more complex example:
+
+```php
+function legacyLibsUsage() {
+  /**
+   * @warning      Don't use legacy libs
+   * @filter $file (legacy\.lib)
+   */
+  any_legacy_libs_usage: {
+    require ${'file:str'};
+    require_once ${'file:str'};
+    include ${'file:str'};
+    include_once ${'file:str'};
+
+    require __DIR__ . ${'file:str'};
+    require_once __DIR__ . ${'file:str'};
+    include __DIR__ . ${'file:str'};
+    include_once __DIR__ . ${'file:str'};
+  }
+}
+```
+
+This regular expression (legacy\.lib) will match any line that contains the substring legacy.lib in the substituted expression of the $file
 
 #### Underline location (`@location`)
 

src/linter/root.go

@@ -1893,7 +1893,14 @@ func (d *rootWalker) runRule(n ir.Node, sc *meta.Scope, rule *rules.Rule) bool {
 		matched = true
 	} else {
 		for _, filterSet := range rule.Filters {
-			if d.checkFilterSet(&m, sc, filterSet) {
+
+			filterMatched, errorFilterSet := d.checkFilterSet(&m, sc, filterSet)
+			if errorFilterSet != nil {
+				d.Report(n, rule.Level, rule.Name, "%s", errorFilterSet)
+				return true
+			}
+
+			if filterMatched {
 				matched = true
 				break
 			}
@@ -1950,7 +1957,7 @@ func (d *rootWalker) checkTypeFilter(wantType *phpdoc.Type, sc *meta.Scope, nn i
 	return rules.TypeIsCompatible(wantType.Expr, haveType.Expr)
 }
 
-func (d *rootWalker) checkFilterSet(m *phpgrep.MatchData, sc *meta.Scope, filterSet map[string]rules.Filter) bool {
+func (d *rootWalker) checkFilterSet(m *phpgrep.MatchData, sc *meta.Scope, filterSet map[string]rules.Filter) (bool, error) {
 	// TODO: pass custom types here, so both @type and @pure predicates can use it.
 
 	for name, filter := range filterSet {
@@ -1960,24 +1967,27 @@ func (d *rootWalker) checkFilterSet(m *phpgrep.MatchData, sc *meta.Scope, filter
 		}
 
 		if !d.checkTypeFilter(filter.Type, sc, nn) {
-			return false
+			return false, nil
 		}
 		if filter.Pure && !solver.SideEffectFree(d.scope(), d.ctx.st, nil, nn) {
-			return false
+			return false, nil
 		}
 		if filter.Regexp != nil {
 			switch v := nn.(type) {
 			case *ir.SimpleVar:
 				if !filter.Regexp.MatchString(v.Name) {
-					return false
+					return false, nil
 				}
 			case *ir.String:
 				if !filter.Regexp.MatchString(v.Value) {
-					return false
+					return false, nil
 				}
+			default:
+				// logical paradox: we handled it, but does not support that's why here false
+				return false, fmt.Errorf("applying @filter for construction '%s' does not support. Current supported capturing types are str and var", d.nodeText(nn))
 			}
 		}
 	}
 
-	return true
+	return true, nil
 }

src/rules/parser.go

@@ -363,7 +363,7 @@ func (p *parser) parseRuleInfo(st ir.Node, labelStmt ir.Node, proto *Rule) (Rule
 				return rule, p.errorf(st, "@filter param must be a phpgrep variable")
 			}
 			name = strings.TrimPrefix(name, "$")
-			found := p.checkForVariableInPattern(name, patternStmt, verifiedVars)
+			found := p.filterByPattern(name, patternStmt, verifiedVars)
 			if !found {
 				return rule, p.errorf(st, "@filter contains a reference to a variable %s that is not present in the pattern", name)
 			}
@@ -558,3 +558,35 @@ func (p *parser) checkForVariableInPattern(name string, pattern ir.Node, verifie
 
 	return found
 }
+
+func (p *parser) filterByPattern(name string, pattern ir.Node, verifiedVars map[string]struct{}) bool {
+	if _, ok := verifiedVars[name]; ok {
+		return true
+	}
+
+	found := irutil.FindWithPredicate(&ir.SimpleVar{Name: name}, pattern, func(what ir.Node, cur ir.Node) bool {
+		// we can capture anything: vars, const, int and etc: see more in phpgrep doc. Example:
+		/*
+		   @filter  $file ^var
+		             ^^^^^^ <- captured
+		   callApi(${'file:str'});
+		              ^^^^^^^^ <- patternForFound
+		*/
+		if s, ok := cur.(*ir.Var); ok {
+			if s, ok := s.Expr.(*ir.String); ok {
+				captured := what.(*ir.SimpleVar).Name
+				patternForFound := s.Value
+
+				return strings.HasPrefix(patternForFound, captured)
+			}
+		}
+
+		return false
+	})
+
+	if found {
+		verifiedVars[name] = struct{}{}
+	}
+
+	return found
+}

src/tests/rules/rules_test.go

@@ -780,6 +780,214 @@ function testMultipleEndpoints() {
 	test.RunRulesTest()
 }
 
+func TestFilterLegacyLibsUsageMatches(t *testing.T) {
+	rfile := `<?php
+function legacyLibsUsage() {
+  /**
+   * @warning      Don't use legacy libs
+   * @filter $file (legacy\.lib)
+   */
+  any_legacy_libs_usage: {
+    require ${'file:str'};
+    require_once ${'file:str'};
+    include ${'file:str'};
+    include_once ${'file:str'};
+
+    require __DIR__ . ${'file:str'};
+    require_once __DIR__ . ${'file:str'};
+    include __DIR__ . ${'file:str'};
+    include_once __DIR__ . ${'file:str'};
+  }
+}
+`
+	test := linttest.NewSuite(t)
+	test.RuleFile = rfile
+	test.AddFile(`<?php
+function testLegacyUsage() {
+  // Should match (because the have substring "legacy.lib")
+  require "legacy.lib.php";
+  include "legacy.lib.inc";
+  include_once __DIR__ . "legacy.lib";
+  
+  // should not match because has not substring
+  require_once "modern.lib.php";
+  
+  // should match
+  require __DIR__ . "other.lib";
+}
+`)
+	test.Expect = []string{
+		"Don't use legacy libs", // for require "legacy.lib.php"
+		"Don't use legacy libs", // for include "legacy.lib.inc"
+		"Don't use legacy libs", // for include_once __DIR__ . "legacy.lib"
+	}
+	test.RunRulesTest()
+}
+
+func TestFilterInsecureUrlExpr(t *testing.T) {
+	rfile := `<?php
+function insecureUrl() {
+  /**
+   * @warning Use secure URLs
+   * @filter $url ^http://
+   */
+  callApi(${ "url:expr" });
+}
+`
+	test := linttest.NewSuite(t)
+	test.RuleFile = rfile
+	test.AddFile(`<?php
+function testInsecureUrl() {
+  callApi("http://example.com");  // match, because ^http://
+  callApi("https://secure.com");  // should not match
+}
+`)
+	test.Expect = []string{
+		"Use secure URLs",
+	}
+	test.RunRulesTest()
+}
+
+func TestFilterStrVarCatching(t *testing.T) {
+	rfile := `<?php
+function catchingDiffTypes() {
+  /**
+   * @warning str '$name' warning
+   * @filter  $name ^str_name$
+   */
+  callFunc(${'name:str'});
+
+  /**
+   * @warning var '$name' warning
+   * @filter  $name ^var_name$
+   */
+  callFunc(${'name:var'});
+
+}
+`
+	test := linttest.NewSuite(t)
+	test.RuleFile = rfile
+	test.AddFile(`<?php
+  // For a string literal: captured without quotes inside the filter
+  callFunc("str_name"); // str '"str_name"' warning
+
+  // For variable: name without dollar sign
+  $var_name = "";
+  callFunc($var_name); // var '$var_name' warning
+`)
+	test.Expect = []string{
+		`str '"str_name"' warning`,
+		`var '$var_name' warning`,
+	}
+	test.RunRulesTest()
+}
+
+func TestFilterMultiCatching(t *testing.T) {
+	rfile := `<?php
+function catchingDiffTypes() {
+  /**
+   * @warning str '$name' warning
+   * @filter  $name ^str_name$
+   */
+  callFunc(${'name:str'});
+
+  /**
+   * @warning var '$name' warning
+   * @filter  $name ^var_name$
+   */
+  callFunc(${'name:var'});
+
+  /**
+   * @warning const '$name' warning
+   * @filter  $name _name^
+   */
+  callFunc(${'name:const'});
+
+  /**
+   * @warning call '$name' warning
+   * @filter  $name _name^
+   */
+  callFunc(${'name:call'});
+
+  /**
+   * @warning int '$name' warning
+   * @filter  $name ^42$
+   */
+  callFunc(${'name:int'});
+
+  /**
+   * @warning float '$name' warning
+   * @filter  $name ^3\.14$
+   */
+  callFunc(${'name:float'});
+
+  /**
+   * @warning char '$name' warning
+   * @filter  $name ^a$
+   */
+  callFunc(${'name:char'});
+
+  /**
+   * @warning func '$name' warning
+   * @filter  $name _function
+   */
+  callFunc(${'name:func'});
+
+  /**
+   * @warning expr '$name' warning
+   * @filter  $name ^\$a\+1$
+   */
+  callFunc(${'name:expr'});
+}`
+
+	test := linttest.NewSuite(t)
+	test.RuleFile = rfile
+	test.AddFile(`<?php
+  // For a string literal: captured without quotes inside the filter
+  callFunc("str_name"); // str '"str_name"' warning
+
+  // For variable: name without dollar sign
+  $var_name = "";
+  callFunc($var_name); // var '$var_name' warning
+
+  // For a function call: a textual representation of the identifier to be called
+  callFunc(funcNane()); // call 'funcNane()' warning
+
+  // For constant: output as is
+  const const_name = "";
+  callFunc(const_name); // const 'const_name' warning
+
+  // For an integer literal
+  callFunc(42); // int '42' warning
+
+  // For a floating point number. Important: the escaping of the dot
+  callFunc(3.14); // float '3.14' warning
+
+  // For a character literal: a string of length 1
+  callFunc('a'); // char 'a' warning
+
+  // For an anonymous function: the text representation must contain the substring "function"
+  callFunc(function() {}); // func 'function() {}' warning
+
+  // For an arbitrary expression: after normalization, spaces are removed
+  $a = 10;
+  callFunc($a + 1); // expr '$a+1' warning
+`)
+	test.Expect = []string{
+		`str '"str_name"' warning`,
+		`var '$var_name' warning`,
+		`applying @filter for construction 'funcNane()' does not support. Current supported capturing types are str and var`,
+		`applying @filter for construction 'const_name' does not support. Current supported capturing types are str and var`,
+		`applying @filter for construction '42' does not support. Current supported capturing types are str and var`,
+		`applying @filter for construction '3.14' does not support. Current supported capturing types are str and var`,
+		`char ''a'' warning`,
+		`applying @filter for construction 'function() {}' does not support. Current supported capturing types are str and var`,
+		`applying @filter for construction '$a + 1' does not support. Current supported capturing types are str and var`,
+	}
+	test.RunRulesTest()
+
+}
+
 func TestFilterVariableNoWarning(t *testing.T) {
 	rfile := `<?php
 function variableEndpointSafe() {