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

Possible insecure pointer conversion in yr_set_configuration() leading to global-buffer-overflow #1616

Closed
0xdd96 opened this issue Dec 15, 2021 · 3 comments

Comments

@0xdd96
Copy link

0xdd96 commented Dec 15, 2021

version: master (commit 822e532)
command: yara $FILE strings
PoC is a file that can contain any string, such as "hello". Here is the trace reported by ASAN:

root@d8a714203f6e:/path_to_yara/bin# ./yara PoC strings
=================================================================
==25743==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000763280 at pc 0x00000052260c bp 0x7ffda7b01e60 sp 0x7ffda7b01e58
READ of size 8 at 0x000000763280 thread T0
    #0 0x52260b in yr_set_configuration /path_to_yara/libyara/libyara.c:395:26
    #1 0x4eef83 in main /path_to_yara/cli/yara.c:1394:3
    #2 0x7f7d5f39d83f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
    #3 0x41b5e8 in _start (/path_to_yara/build/bin/yara+0x41b5e8)

0x000000763284 is located 0 bytes to the right of global variable 'max_process_memory_chunk' defined in 'cli/yara.c:167:12' (0x763280) of size 4
SUMMARY: AddressSanitizer: global-buffer-overflow /path_to_yara/libyara/libyara.c:395:26 in yr_set_configuration
Shadow bytes around the buggy address:
  0x0000800e4600: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x0000800e4610: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x0000800e4620: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x0000800e4630: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x0000800e4640: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 00 00
=>0x0000800e4650:[04]f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
  0x0000800e4660: 01 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
  0x0000800e4670: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
  0x0000800e4680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000800e4690: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0000800e46a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==25743==ABORTING

Commit 605b2ed introduced a configuration case called YR_CONFIG_MAX_PROCESS_MEMORY_CHUNK , which will treat the void *src pointer as a uint64_t* pointer (64bit). The dereferece operation after this will read 64bits from src.

yara/libyara/libyara.c

Lines 381 to 403 in 605b2ed

YR_API int yr_set_configuration(YR_CONFIG_NAME name, void *src)
{
if (src == NULL)
return ERROR_INTERNAL_FATAL_ERROR;
switch (name)
{ // lump all the cases using same types together in one cascade
case YR_CONFIG_STACK_SIZE:
case YR_CONFIG_MAX_STRINGS_PER_RULE:
case YR_CONFIG_MAX_MATCH_DATA:
yr_cfgs[name].ui32 = *(uint32_t *) src;
break;
case YR_CONFIG_MAX_PROCESS_MEMORY_CHUNK:
yr_cfgs[name].ui64 = *(uint64_t *) src;
break;
default:
return ERROR_INTERNAL_FATAL_ERROR;
}
return ERROR_SUCCESS;
}

Note that, in cli/yara.c, a pointer to the 32bit integer max_process_memory_chunk is passed to yr_set_configuration. As a result, yr_cfgs[name].ui64 = *(uint64_t *) src; will read 64bits from a 32bit variable. This caused the ERROR reported by ASAN.

yara/cli/yara.c

Line 166 in 605b2ed

static int max_process_memory_chunk = DEFAULT_MAX_PROCESS_MEMORY_CHUNK;

yara/cli/yara.c

Lines 1371 to 1372 in 605b2ed

yr_set_configuration(
YR_CONFIG_MAX_PROCESS_MEMORY_CHUNK, &max_process_memory_chunk);

A potential damage of this is that an attacker who obtains control of max_process_memory_chunk's next 4 bytes in the memory can further set the higher 32bits of yr_cfgs[name].ui64 to arbitrary values and launch exhaustive attacks.

plusvic added a commit that referenced this issue Dec 17, 2021
This is a more comprehensive fix than #1617, it adds new functions to the API for getting/setting uint32 and uint64 settings.  Using these functions is preferable over calling `yr_(get|set)_configuration` directly.
@plusvic
Copy link
Member

plusvic commented Dec 17, 2021

@dandanxu96 please check this #1621

@0xdd96
Copy link
Author

0xdd96 commented Dec 19, 2021

I checked pr #1621 and confirm that this commit is a valid fix for #1616.

@plusvic
Copy link
Member

plusvic commented Dec 20, 2021

Thanks!

plusvic added a commit that referenced this issue Dec 20, 2021
This is a more comprehensive fix than #1617, it adds new functions to the API for getting/setting uint32 and uint64 settings.  Using these functions is preferable over calling `yr_(get|set)_configuration` directly.
@plusvic plusvic closed this as completed Dec 20, 2021
tarterp pushed a commit to mandiant/yara that referenced this issue Mar 31, 2022
This is a more comprehensive fix than VirusTotal#1617, it adds new functions to the API for getting/setting uint32 and uint64 settings.  Using these functions is preferable over calling `yr_(get|set)_configuration` directly.
maximelb pushed a commit to refractionPOINT/yara that referenced this issue Nov 17, 2022
This is a more comprehensive fix than VirusTotal#1617, it adds new functions to the API for getting/setting uint32 and uint64 settings.  Using these functions is preferable over calling `yr_(get|set)_configuration` directly.
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

2 participants