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

Heap based buffer overflow in njs_module.c #187

Closed
l0kihardt opened this issue Jun 29, 2019 · 2 comments
Closed

Heap based buffer overflow in njs_module.c #187

l0kihardt opened this issue Jun 29, 2019 · 2 comments
Assignees
Labels

Comments

@l0kihardt
Copy link

l0kihardt commented Jun 29, 2019

env

ubuntu 18.04
njs 0feca92
gcc version 7.4.0 (Ubuntu 7.4.0-1ubuntu1~18.04.1)

bug

So, actually this is a logic bug, happened under an really interesting circumstance.
the buggy function is the njs_module_read in the file njs_module.c

if (fstat(fd, &sb) == -1) {
        goto fail;
    }

    text->length = nxt_length(NJS_MODULE_START);

    if (S_ISREG(sb.st_mode) && sb.st_size) {
        text->length += sb.st_size;
    }

    text->length += nxt_length(NJS_MODULE_END);

    text->start = nxt_mp_alloc(vm->mem_pool, text->length);
    if (text->start == NULL) {
        goto fail;
    }

    p = nxt_cpymem(text->start, NJS_MODULE_START, nxt_length(NJS_MODULE_START));

    n = read(fd, p, sb.st_size);

as you can see, it read the sb.st_size and sb.st_mode with function fstat. However if we dont provide a common .js file. the S_ISREG(sb.st_mode) will be 0. text->length wont be updated.
and read(fd, p, sb.st_size); still read sb.st_size bytes into the p. p is on the heap. So we can overflow to the next chunk on the heap....

poc

import crypto from '/'
@xeioex xeioex self-assigned this Jul 1, 2019
@xeioex xeioex added the bug label Jul 1, 2019
@abergmann
Copy link

CVE-2020-19692 was assigned to this issue.

@amznsina
Copy link

Hey @abergmann on what basis is the CVE assigned to this issue and what assessment was conducted to determine a remote attacker can do this exploit?

These statements from nginx devs seem to contradict the need for a CVE and definitely the idea that a remote attacker could ever exploit it.

#188 (comment)

#188 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants