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

Crash when calling mm_mapopt_update() due to NULL pointer dereference #1147

Closed
blawrence-ont opened this issue Jan 12, 2024 · 0 comments · Fixed by #1154
Closed

Crash when calling mm_mapopt_update() due to NULL pointer dereference #1147

blawrence-ont opened this issue Jan 12, 2024 · 0 comments · Fixed by #1154

Comments

@blawrence-ont
Copy link
Contributor

Crash seen when calling mm_mapopt_update() due to NULL pointer dereference. Minimised repro case:

#include "minimap.h"

int main()
{
    mm_idxopt_t idx_opt;
    mm_mapopt_t map_opt;
    mm_set_opt(0, &idx_opt, &map_opt);
    mm_set_opt("map-hifi", &idx_opt, &map_opt);

    const char *seq = "ACGT";
    const char *name = "query";
    mm_idx_t *index = mm_idx_str(idx_opt.w, idx_opt.k, 0, idx_opt.bucket_bits, 1, &seq, &name);
    mm_mapopt_update(&map_opt, index);
}

This doesn't repro on every system due to implementation defined behaviour of malloc(0). However ASAN can show the issue on macOS (Linux's glibc doesn't have the same allocation strategy so ASAN doesn't highlight the issue there):

$ git clone https://github.com/lh3/minimap2.git
$ cd minimap2
$ make asan=1 -j 8
$ cc -fsanitize=address libminimap2.a -lm -lz -lpthread repro.c
$ ./a.out
==93948==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x000106e006b0 at pc 0x000104114c94 bp 0x00016bcf2f50 sp 0x00016bcf2f48
READ of size 4 at 0x000106e006b0 thread T0
    #0 0x104114c90 in ks_ksmall_uint32_t misc.c
    #1 0x10411edfc in mm_idx_cal_max_occ index.c:205
    #2 0x104119d30 in mm_mapopt_update options.c:72
    #3 0x104127af0 in main+0x3d4 (a.out:arm64+0x10001baf0)
    #4 0x180c610dc  (<unknown module>)

0x000106e006b1 is located 0 bytes after 1-byte region [0x000106e006b0,0x000106e006b1)
allocated by thread T0 here:
    #0 0x1049e3244 in wrap_malloc+0x94 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x53244)
    #1 0x10411eec0 in mm_idx_cal_max_occ index.c:196
    #2 0x104119d30 in mm_mapopt_update options.c:72
    #3 0x104127af0 in main+0x3d4 (a.out:arm64+0x10001baf0)
    #4 0x180c610dc  (<unknown module>)

The allocation can be traced back to the line:

a = (uint32_t*)malloc(n * 4);

At this point n == 0 and hence a cannot be dereferenced, however ks_ksmall_uint32_t() does this here (since both high and low are 0):

if (high <= low) return *k;

Suggestion was made for the following in mm_idx_cal_max_occ() to match the f <= 0. check:

+if (n == 0) return INT32_MAX;
 a = (uint32_t*)malloc(n * 4);
blawrence-ont added a commit to blawrence-ont/minimap2 that referenced this issue Jan 24, 2024
If the allocated region is 0 bytes then it's unsafe to dereference it.

Fixes lh3#1147.
@lh3 lh3 closed this as completed in #1154 Jan 24, 2024
lh3 pushed a commit that referenced this issue Jan 24, 2024
If the allocated region is 0 bytes then it's unsafe to dereference it.

Fixes #1147.
tijyojwad pushed a commit to nanoporetech/dorado that referenced this issue Apr 2, 2024
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 a pull request may close this issue.

1 participant