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

Fix null derefrences while loading compiled rules #1727

Merged
merged 3 commits into from
Jun 14, 2022
Merged

Fix null derefrences while loading compiled rules #1727

merged 3 commits into from
Jun 14, 2022

Conversation

sudhackar
Copy link
Contributor

@sudhackar sudhackar commented Jun 14, 2022

While reading compiled rules file - there are a couple of places where a NULL dereference could happen.

  1. yr_arena_get_ptr could return NULL which is not checked in

    yara/libyara/rules.c

    Lines 333 to 343 in 929af6e

    YR_SUMMARY* summary = (YR_SUMMARY*) yr_arena_get_ptr(
    arena, YR_SUMMARY_SECTION, 0);
    // Now YR_RULES relies on this arena, let's increment the arena's
    // reference count so that if the original owner of the arena calls
    // yr_arena_destroy the arena is not destroyed.
    yr_arena_acquire(arena);
    new_rules->arena = arena;
    new_rules->num_rules = summary->num_rules;
    new_rules->num_strings = summary->num_strings;
  2. b->data could be NULL in yr_arena_load_stream in

    yara/libyara/arena.c

    Lines 597 to 610 in 929af6e

    YR_ARENA_BUFFER* b = &new_arena->buffers[reloc_ref.buffer_id];
    if (reloc_ref.buffer_id >= new_arena->num_buffers ||
    reloc_ref.offset > b->used - sizeof(void*))
    {
    yr_arena_release(new_arena);
    return ERROR_CORRUPT_FILE;
    }
    YR_ARENA_REF ref;
    memcpy(&ref, b->data + reloc_ref.offset, sizeof(ref));
    void* reloc_ptr = yr_arena_ref_to_ptr(new_arena, &ref);

I can attach some sample testcases that could trigger these code paths.

@google-cla
Copy link

google-cla bot commented Jun 14, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@sudhackar
Copy link
Contributor Author

For 1 the backtrace looked like

AddressSanitizer:DEADLYSIGNAL
=================================================================
==6285==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fe328a8375f bp 0x7fff6df4c210 sp 0x7fff6df4c130 T0)
==6285==The signal is caused by a READ memory access.
==6285==Hint: address points to the zero page.
    #0 0x7fe328a8375f in yr_rules_from_arena /home/test/test/yara/libyara/rules.c:342:35
    #1 0x7fe328a83b6f in yr_rules_load_stream /home/test/test/yara/libyara/rules.c:378:3
    #2 0x4cf38b in main /home/test/test/yara/cli/yara.c:1452:16
    #3 0x7fe327988c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310
    #4 0x41d1c9 in _start (/home/test/test/yara/.libs/yara+0x41d1c9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/test/test/yara/libyara/rules.c:342:35 in yr_rules_from_arena
==6285==ABORTING

For 2

AddressSanitizer:DEADLYSIGNAL
=================================================================
==6330==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000004 (pc 0x7fe03ca88bdd bp 0x7ffdee203ad0 sp 0x7ffdee203860 T0)
==6330==The signal is caused by a READ memory access.
==6330==Hint: address points to the zero page.
    #0 0x7fe03ca88bdd in yr_arena_load_stream /home/test/test/yara/libyara/arena.c:608:5
    #1 0x7fe03cb13ade in yr_rules_load_stream /home/test/test/yara/libyara/rules.c:374:3
    #2 0x4cf38b in main /home/test/test/yara/cli/yara.c:1452:16
    #3 0x7fe03ba18c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310
    #4 0x41d1c9 in _start (/home/test/test/yara/.libs/yara+0x41d1c9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/test/test/yara/libyara/arena.c:608:5 in yr_arena_load_stream
==6330==ABORTING

I can push the test cases to
https://github.com/VirusTotal/yara/tree/master/tests/data if needed

@sudhackar
Copy link
Contributor Author

Pushed an additional change for this

AddressSanitizer:DEADLYSIGNAL
=================================================================
==4064==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f1fa20e0461 bp 0x7fffc226ca50 sp 0x7fffc226c208 T0)
==4064==The signal is caused by a READ memory access.
==4064==Hint: address points to the zero page.
    #0 0x7f1fa20e0461  /build/glibc-CVJwZb/glibc-2.27/string/../sysdeps/x86_64/multiarch/strlen-avx2.S:65
    #1 0x485412 in strdup (/home/test/test/yara/.libs/yara+0x485412)
    #2 0x7f1fa303d7f5 in yr_object_create /home/test/test/yara/libyara/object.c:94:21
    #3 0x7f1fa303ef28 in yr_object_from_external_variable /home/test/test/yara/libyara/object.c:279:12
    #4 0x7f1fa3074f73 in yr_scanner_create /home/test/test/yara/libyara/scanner.c:263:5
    #5 0x4cfe80 in main /home/test/test/yara/cli/yara.c:1600:14
    #6 0x7f1fa1f73c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310
    #7 0x41d1c9 in _start (/home/test/test/yara/.libs/yara+0x41d1c9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /build/glibc-CVJwZb/glibc-2.27/string/../sysdeps/x86_64/multiarch/strlen-avx2.S:65
==4064==ABORTING

libyara/object.c Outdated Show resolved Hide resolved
@plusvic plusvic merged commit e23ac0d into VirusTotal:master Jun 14, 2022
plusvic added a commit that referenced this pull request Jun 30, 2022
* Fix null derefrences while loading compiled rules

* Fix nulldereference in yr_object_create

* Fix assert to explicitly catch null identifier in yr_object_create
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

Successfully merging this pull request may close these issues.

2 participants