Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No check if "yyscanner" variable has not been initialized in yylex_init() #1674

Closed
1ndahous3 opened this issue Apr 2, 2022 · 1 comment
Closed

Comments

@1ndahous3
Copy link
Contributor

yylex_init() may fail to init the yyscanner variable due to memory allocation failure and will return 1:

yara/libyara/lexer.c

Lines 3279 to 3291 in 2d22a4a

int yylex_init(yyscan_t* ptr_yy_globals)
{
if (ptr_yy_globals == NULL){
errno = EINVAL;
return 1;
}
*ptr_yy_globals = (yyscan_t) yyalloc ( sizeof( struct yyguts_t ), NULL );
if (*ptr_yy_globals == NULL){
errno = ENOMEM;
return 1;
}

In all yr_lex_parse_rules_*() and yr_parse_*() routines, the retcode isn't saved and/or checked:

yara/libyara/hex_lexer.l

Lines 265 to 269 in e1360f6

yylex_init(&yyscanner);
yyset_extra(*re_ast, yyscanner);
yy_scan_string(hex_string, yyscanner);
yyparse(yyscanner, &lex_env);
yylex_destroy(yyscanner);

And later we can get a nulltpr dereference for the yyscanner variable:
image

Detected by the Application Verifier (Windows) with the low resource simulation feature:

@plusvic plusvic closed this as completed in b47b60d Apr 4, 2022
@1ndahous3
Copy link
Contributor Author

@plusvic it's important to do a full cleanup on re_ast in case of yylex_init() != 0 if the caller uses that value later.

In b47b60d you added a retcode check and yr_re_ast_destroy(*re_ast); in yr_parse_hex_string() and yr_parse_re_string() , but didn't add *re_ast = NULL;, so for example yr_parser_reduce_string_declaration() will read an invalid address in the exit step.

yara/libyara/parser.c

Lines 738 to 745 in 2d22a4a

if (modifier.flags & STRING_FLAGS_HEXADECIMAL)
result = yr_re_parse_hex(str->c_string, &re_ast, &re_error);
else if (modifier.flags & STRING_FLAGS_REGEXP)
result = yr_re_parse(str->c_string, &re_ast, &re_error);
else
result = yr_base64_ast_from_string(str, modifier, &re_ast, &re_error);
if (result != ERROR_SUCCESS)

yara/libyara/parser.c

Lines 898 to 907 in 2d22a4a

_exit:
if (re_ast != NULL)
yr_re_ast_destroy(re_ast);
if (remainder_re_ast != NULL)
yr_re_ast_destroy(remainder_re_ast);
return result;
}

gotcha!
image

plusvic added a commit that referenced this issue Apr 6, 2022
plusvic added a commit that referenced this issue Apr 6, 2022
… to NULL.

As @1ndahous3 highlighted in #1674, not setting the pointer to NULL leads to a dangling pointer.
plusvic added a commit that referenced this issue Apr 6, 2022
… to NULL.

As @1ndahous3 highlighted in #1674, not setting the pointer to NULL leads to a dangling pointer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant