VirusTotal/yara
1
0
Fork 0
mirror of https://github.com/VirusTotal/yara.git synced 2026-10-12 02:10:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix some compiler edge cases. (#1709)

Browse Source 
* Fix some compiler edge cases.

Add a warning when "0 of them" is used. Point to some new documentation that
explains why "none of them" is preferred.

Handle some edge cases, which are all now errors:

* -1 of them
* "foo" of them
* /foo/ of them

And this one is also an error if the external variable "x" is a negative integer
or a string:

* x of them

I've also made sure this works with a construct like "0 + x of them" where x is
defined as a negative integer.

Add test cases for as many of these as I can. I don't have test cases for the
external variable case, but it does work as expected. Here is the output of a
rule using external variables where the variable is defined to be 1, 0, -1 and
foo:

```
wxs@mbp yara % cat rules/test.yara
rule a {
  strings:
    $a = "FreeBSD"
  condition:
    x of them
}
wxs@mbp yara % ./yara -d x=1 rules/test.yara /bin/ls
a /bin/ls
wxs@mbp yara % ./yara -d x=0 rules/test.yara /bin/ls
warning: rule "a" in rules/test.yara(5): Consider using "none" keyword, it is less ambiguous. Please see https://yara.readthedocs.io/en/stable/writingrules.html#sets-of-strings-1 for an explanation.
wxs@mbp yara % ./yara -d x=-1 rules/test.yara /bin/ls
error: rule "a" in rules/test.yara(5): invalid value in condition: "-1"
wxs@mbp yara % ./yara -d x=foo rules/test.yara /bin/ls
error: rule "a" in rules/test.yara(5): invalid value in condition: "string in for_expression is invalid"
wxs@mbp yara %
```

* More compiler warnings and errors.

If you specify "2 of ($a)" it is now a warning because it will always evaluate
to false, and is quite possibly not what you wanted to write. This is true for
string sets, rule sets and string sets in a range like "2 of ($a) in (0..10)".

If you are using an integer range it is now an error if the lower bound is
greater than the upper bound.

To support rule sets I needed to modify yr_parser_emit_pushes_for_rules() to
use a count parameter, which it will update with the number of pushed rules.
This is identical to how yr_parser_emit_pushes_for_strings() works.

While I'm here, remove the link from the warning message for the "none" keyword.
This commit is contained in:
Wesley Shields
2022-05-17 18:20:57 +02:00
committed by GitHub
parent 19363ee85e
commit a5903e1d24
10 changed files with 963 additions and 597 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/writingrules.rst
+12
View File
@@ -1200,6 +1200,18 @@ The keywords ``any``, ``all`` and ``none`` can be used as well.
1 of ($*) // same that "any of them"
none of ($b*) // zero of the set of strings that start with "$b"
.. warning:: Due to the way YARA works internally, using "0 of them" is an
ambiguous part of the language which should be avoided in favor of "none
of them". To understand this, consider the meaning of "2 of them", which
is true if 2 or more of the strings match. Historically, "0 of them"
followed this principle and would evaluate to true if at least one of the
strings matched. This ambiguity is resolved in YARA 4.3.0 by making "0 of
them" evaluate to true if exactly 0 of the strings match. To improve on
the situation and make the intent clear, it is encouraged to use "none" in
place of 0. By not using an integer it is easier to reason about the meaning
of "none of them" without the historical understanding of "at least 0"
clouding the issue.
Starting with YARA 4.2.0 it is possible to express a set of strings in an
integer range, like this:
libyara/compiler.c
+7
View File
@@ -1041,6 +1041,13 @@ YR_API char* yr_compiler_get_error_message(
"rule identifier \"%s\" matches previously used wildcard rule set",
compiler->last_error_extra_info);
break;
case ERROR_INVALID_VALUE:
snprintf(
buffer,
buffer_size,
"invalid value in condition: \"%s\"",
compiler->last_error_extra_info);
break;
}
return buffer;
libyara/grammar.c
+682 -576
View File
File diff suppressed because it is too large Load Diff
libyara/grammar.h
+1 -1
View File
@@ -189,7 +189,7 @@ extern int yara_yydebug;
#if ! defined YYSTYPE && ! defined YYSTYPE_IS_DECLARED
union YYSTYPE
{
#line 336 "grammar.y"
#line 341 "grammar.y"
YR_EXPRESSION expression;
SIZED_STRING* sized_string;
libyara/grammar.y
+103 -11
View File
@@ -286,19 +286,24 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
%type <integer> integer_set
%type <integer> integer_enumeration
%type <integer> for_expression
%type <integer> rule_modifier
%type <integer> rule_modifiers
%type <integer> string_enumeration
%type <integer> string_enumeration_item
%type <integer> string_set
%type <integer> for_iteration
%type <integer> rule_enumeration
%type <integer> rule_enumeration_item
%type <integer> rule_set
%type <expression> primary_expression
%type <expression> boolean_expression
%type <expression> expression
%type <expression> identifier
%type <expression> regexp
%type <expression> for_expression
%type <expression> for_quantifier
%type <c_string> arguments
%type <c_string> arguments_list
@@ -1687,12 +1692,22 @@ expression
}
| for_expression _OF_ string_set
{
if ($1.type == EXPRESSION_TYPE_INTEGER && $1.value.integer > $3)
{
yywarning(yyscanner,
"expression always false - requesting %lld of %lld.", $1.value.integer, $3);
}
yr_parser_emit_with_arg(yyscanner, OP_OF, OF_STRING_SET, NULL, NULL);
$$.type = EXPRESSION_TYPE_BOOLEAN;
}
| for_expression _OF_ rule_set
{
if ($1.type == EXPRESSION_TYPE_INTEGER && $1.value.integer > $3)
{
yywarning(yyscanner,
"expression always false - requesting %lld of %lld.", $1.value.integer, $3);
}
yr_parser_emit_with_arg(yyscanner, OP_OF, OF_RULE_SET, NULL, NULL);
$$.type = EXPRESSION_TYPE_BOOLEAN;
@@ -1737,6 +1752,12 @@ expression
}
| for_expression _OF_ string_set _IN_ range
{
if ($1.type == EXPRESSION_TYPE_INTEGER && $1.value.integer > $3)
{
yywarning(yyscanner,
"expression always false - requesting %lld of %lld.", $1.value.integer, $3);
}
yr_parser_emit(yyscanner, OP_OF_FOUND_IN, NULL);
$$.type = EXPRESSION_TYPE_BOOLEAN;
@@ -2106,6 +2127,18 @@ range
result = ERROR_WRONG_TYPE;
}
// If we can statically determine lower and upper bounds, ensure
// lower < upper. Check for upper bound here because some things (like
// string count) are EXPRESSION_TYPE_INTEGER.
if ($2.value.integer != YR_UNDEFINED &&
$4.value.integer != YR_UNDEFINED &&
$2.value.integer > $4.value.integer)
{
yr_compiler_set_error_extra_info(
compiler, "range lower bound must be greater than upper bound");
result = ERROR_INVALID_VALUE;
}
fail_if_error(result);
}
;
@@ -2214,12 +2247,15 @@ rule_set
yr_parser_emit_push_const(yyscanner, YR_UNDEFINED);
}
rule_enumeration ')'
{
$$ = $3;
}
;
rule_enumeration
: rule_enumeration_item
| rule_enumeration ',' rule_enumeration_item
: rule_enumeration_item { $$ = $1; }
| rule_enumeration ',' rule_enumeration_item { $$ = $1 + $3; }
;
@@ -2254,9 +2290,12 @@ rule_enumeration_item
yr_free($1);
fail_if_error(result);
$$ = 1;
}
| _IDENTIFIER_ '*'
{
int count = 0;
YR_NAMESPACE* ns = (YR_NAMESPACE*) yr_arena_get_ptr(
compiler->arena,
YR_NAMESPACES_TABLE,
@@ -2268,10 +2307,12 @@ rule_enumeration_item
ns->name,
1);
int result = yr_parser_emit_pushes_for_rules(yyscanner, $1);
int result = yr_parser_emit_pushes_for_rules(yyscanner, $1, &count);
yr_free($1);
fail_if_error(result);
$$ = count;
}
;
@@ -2279,22 +2320,73 @@ rule_enumeration_item
for_expression
: primary_expression
{
$$ = FOR_EXPRESSION_ANY;
if ($1.type == EXPRESSION_TYPE_INTEGER)
{
if ($1.value.integer == 0)
{
yywarning(yyscanner,
"consider using \"none\" keyword, it is less ambiguous.");
}
if ($1.value.integer < 0)
{
yr_compiler_set_error_extra_info_fmt(compiler,
"%lld", $1.value.integer);
fail_with_error(ERROR_INVALID_VALUE);
}
}
if ($1.type == EXPRESSION_TYPE_STRING)
{
SIZED_STRING* ss = yr_arena_ref_to_ptr(compiler->arena,
&$1.value.sized_string_ref);
// If the expression is an external string variable we need to get
// it some other way.
if (ss != NULL)
{
yr_compiler_set_error_extra_info_fmt(compiler, "%s", ss->c_string);
}
else
{
yr_compiler_set_error_extra_info(compiler,
"string in for_expression is invalid");
}
fail_with_error(ERROR_INVALID_VALUE);
}
if ($1.type == EXPRESSION_TYPE_REGEXP)
{
yr_compiler_set_error_extra_info(compiler,
"regexp in for_expression is invalid");
fail_with_error(ERROR_INVALID_VALUE);
}
$$.value.integer = $1.value.integer;
}
| _ALL_
| for_quantifier
{
$$.value.integer = $1.value.integer;
}
;
for_quantifier
: _ALL_
{
yr_parser_emit_push_const(yyscanner, YR_UNDEFINED);
$$ = FOR_EXPRESSION_ALL;
}
$$.type = EXPRESSION_TYPE_QUANTIFIER;
$$.value.integer = FOR_EXPRESSION_ALL;
}
| _ANY_
{
yr_parser_emit_push_const(yyscanner, 1);
$$ = FOR_EXPRESSION_ANY;
$$.type = EXPRESSION_TYPE_QUANTIFIER;
$$.value.integer = FOR_EXPRESSION_ANY;
}
| _NONE_
{
yr_parser_emit_push_const(yyscanner, 0);
$$ = FOR_EXPRESSION_NONE;
$$.type = EXPRESSION_TYPE_QUANTIFIER;
$$.value.integer = FOR_EXPRESSION_NONE;
}
;
@@ -2468,7 +2560,7 @@ primary_expression
{
case OBJECT_TYPE_INTEGER:
$$.type = EXPRESSION_TYPE_INTEGER;
$$.value.integer = YR_UNDEFINED;
$$.value.integer = $1.value.object->value.i;
break;
case OBJECT_TYPE_FLOAT:
$$.type = EXPRESSION_TYPE_FLOAT;
libyara/include/yara/compiler.h
+8 -7
View File
@@ -42,13 +42,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#define YARA_ERROR_LEVEL_WARNING 1
// Expression type constants are powers of two because they are used as flags.
#define EXPRESSION_TYPE_UNKNOWN 0
#define EXPRESSION_TYPE_BOOLEAN 1
#define EXPRESSION_TYPE_INTEGER 2
#define EXPRESSION_TYPE_STRING 4
#define EXPRESSION_TYPE_REGEXP 8
#define EXPRESSION_TYPE_OBJECT 16
#define EXPRESSION_TYPE_FLOAT 32
#define EXPRESSION_TYPE_UNKNOWN 0
#define EXPRESSION_TYPE_BOOLEAN 1
#define EXPRESSION_TYPE_INTEGER 2
#define EXPRESSION_TYPE_STRING 4
#define EXPRESSION_TYPE_REGEXP 8
#define EXPRESSION_TYPE_OBJECT 16
#define EXPRESSION_TYPE_FLOAT 32
#define EXPRESSION_TYPE_QUANTIFIER 64
// The compiler uses an arena to store the data it generates during the
// compilation. Each buffer in the arena is used for storing a different type
libyara/include/yara/error.h
+1
View File
@@ -106,6 +106,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#define ERROR_BLOCK_NOT_READY 61
#define ERROR_INVALID_PERCENTAGE 62
#define ERROR_IDENTIFIER_MATCHES_WILDCARD 63
#define ERROR_INVALID_VALUE 64
#define GOTO_EXIT_ON_ERROR(x) \
{ \
libyara/include/yara/parser.h
+2 -1
View File
@@ -120,7 +120,8 @@ int yr_parser_emit_pushes_for_strings(
int yr_parser_emit_pushes_for_rules(
yyscan_t yyscanner,
const char* identifier);
const char* identifier,
int *count);
int yr_parser_reduce_external(
yyscan_t yyscanner,
libyara/parser.c
+9 -1
View File
@@ -237,7 +237,10 @@ int yr_parser_emit_pushes_for_strings(
// Emit OP_PUSH_RULE instructions for all rules whose identifier has given
// prefix.
int yr_parser_emit_pushes_for_rules(yyscan_t yyscanner, const char* prefix)
int yr_parser_emit_pushes_for_rules(
yyscan_t yyscanner,
const char* prefix,
int* count)
{
YR_COMPILER* compiler = yyget_extra(yyscanner);
@@ -284,6 +287,11 @@ int yr_parser_emit_pushes_for_rules(yyscan_t yyscanner, const char* prefix)
rule++;
}
if (count != NULL)
{
*count = matching;
}
if (matching == 0)
{
yr_compiler_set_error_extra_info(compiler, prefix);
tests/test-rules.c
+138
View File
@@ -477,6 +477,38 @@ static void test_syntax()
// Test case for issue #1295
assert_error("rule test rule test", ERROR_DUPLICATED_IDENTIFIER);
assert_error(
"rule test { strings: $a = \"a\" condition: -1 of them }",
ERROR_INVALID_VALUE);
assert_error(
"rule test { strings: $a = \"a\" condition: 0 + -1 of them }",
ERROR_INVALID_VALUE);
assert_error(
"rule test { strings: $a = \"a\" condition: for -1 of them: ($) }",
ERROR_INVALID_VALUE);
assert_error(
"rule test { strings: $a = \"a\" condition: for 0 + -1 of them: ($) }",
ERROR_INVALID_VALUE);
assert_error(
"rule test { strings: $a = \"a\" condition: \"foo\" of them }",
ERROR_INVALID_VALUE);
assert_error(
"rule test { strings: $a = \"a\" condition: for \"foo\" of them: ($) }",
ERROR_INVALID_VALUE);
assert_error(
"rule test { strings: $a = \"a\" condition: /foo/ of them }",
ERROR_INVALID_VALUE);
assert_error(
"rule test { strings: $a = \"a\" condition: for /foo/ of them: ($) }",
ERROR_INVALID_VALUE);
YR_DEBUG_FPRINTF(1, stderr, "} // %s()\n", __FUNCTION__);
}
@@ -491,6 +523,93 @@ static void test_anonymous_strings()
YR_DEBUG_FPRINTF(1, stderr, "} // %s()\n", __FUNCTION__);
}
static void test_warnings() {
YR_DEBUG_FPRINTF(1, stderr, "+ %s() {\n", __FUNCTION__);
assert_warning("rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
0 of them \
}");
assert_warning("rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
for 0 of ($a*): ($) \
}");
assert_no_warnings("rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
none of them \
}");
assert_no_warnings("rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
for none of ($a*): ($) \
}");
assert_warning("rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
1 + -1 of them \
}");
assert_warning("rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
2 of them \
}");
assert_warning("rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
2 of ($a) \
}");
assert_warning("rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
2 of ($a*) \
}");
assert_warning("rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
2 of ($a*) in (0..10) \
}");
assert_warning("rule a { \
condition: \
true \
} \
rule b { \
condition: \
2 of (a) \
}");
assert_warning("rule a { \
condition: \
true \
} \
rule b { \
condition: \
2 of (a*) \
}");
YR_DEBUG_FPRINTF(1, stderr, "} // %s()\n", __FUNCTION__);
}
static void test_strings()
{
YR_DEBUG_FPRINTF(1, stderr, "+ %s() {\n", __FUNCTION__);
@@ -2045,6 +2164,24 @@ void test_for()
}",
NULL);
// Lower bound must be less than upper bound, if it can be determined statically.
assert_error(
"import \"tests\" \
rule test { \
condition: \
for any i in (10..1): (i) \
}",
ERROR_INVALID_VALUE);
// If one of the bounds can not be determined statically it isn't an error.
assert_true_rule(
"rule test { \
strings: \
$a = \"AXSERS\" \
condition: \
true or any of them in (0..filesize-100) \
}", TEXT_1024_BYTES);
YR_DEBUG_FPRINTF(1, stderr, "} // %s()\n", __FUNCTION__);
}
@@ -3537,6 +3674,7 @@ static void test_pass(int pass)
test_global_rules();
test_tags();
test_meta();
test_warnings();
#if !defined(USE_NO_PROC) && !defined(_WIN32) && !defined(__CYGWIN__)
test_process_scan();