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

NULL pointer dereference is found in dlo-lhp.c #3140

Closed
iwashiira opened this issue May 11, 2024 · 2 comments
Closed

NULL pointer dereference is found in dlo-lhp.c #3140

iwashiira opened this issue May 11, 2024 · 2 comments

Comments

@iwashiira
Copy link
Contributor

Our fuzzer found null pointer dereference in dlo-lhp.c. in the current main(9ba1504).
PoC is here.
This harness is a modification of api-test-lhp to take input from a file.

#include "libwebsockets.h"
#include "libwebsockets/lws-jpeg.h"

static const lws_surface_info_t ic = {
	.wh_px = { { 600,0 },       { 448,0 } },
	.wh_mm = { { 114,5000000 }, {  82,5000000 } },
};

static lws_displaylist_t displaylist;

static const char * const cb_reasons[] = {
		"LHPCB_CONSTRUCTED",
		"LHPCB_DESTRUCTED",
		"LHPCB_COMPLETE",
		"LHPCB_FAILED",
		"LHPCB_ELEMENT_START",	/* reported at end of <> */
		"LHPCB_ELEMENT_END",
		"LHPCB_CONTENT",
		"LHPCB_COMMENT",
};

static int
dump_atr(lws_dll2_t *d, void *user)
{
	lhp_atr_t *atr = lws_container_of(d, lhp_atr_t, list);
	const char *p = (const char *)&atr[1];

	printf("{ \"%.*s\", \"%.*s\" }, ",
		    (int)atr->name_len, p, (int)atr->value_len, p + atr->name_len + 1);

	return 0;
}

static lws_stateful_ret_t
test_cb(lhp_ctx_t *ctx, char reason)
{
	lhp_pstack_t *ps = lws_container_of(ctx->stack.tail, lhp_pstack_t, list);
	const lcsp_atr_t *a;

	printf("{ %s, %u, \"%.*s\", %u, { ", cb_reasons[(unsigned int)reason], ctx->npos, ctx->npos, ctx->buf, ps->atr.count);

	if (reason == LHPCB_ELEMENT_START || reason == LHPCB_ELEMENT_END) {
		lws_dll2_foreach_safe(&ps->atr, NULL, dump_atr);

		a = lws_css_cascade_get_prop_atr(ctx, LCSP_PROP_DISPLAY);
		if (a)
			lwsl_notice("display: %d\n", a->propval);

		a = lws_css_cascade_get_prop_atr(ctx, LCSP_PROP_COLOR);
		if (a)
			lwsl_notice("color: %d 0x%08X\n", a->propval, a->u.rgba);
		//lwsl_notice("color: active_stz %d, atr %d\n", ctx->active_stanzas.count, ctx->active_atr.count);
		//lws_dll2_foreach_safe(&ctx->active_atr, NULL, dump_css_atr);

		a = ps->css_position;
		if (a)
			lwsl_notice("position: %d\n", a->propval);

		a = ps->css_width;
		if (a)
			lwsl_notice("width: %d.%u\n", a->u.i.whole, a->u.i.frac);

		a = ps->css_height;
		if (a)
			lwsl_notice("height: %d.%u\n", a->u.i.whole, a->u.i.frac);

		a = ps->css_pos[CCPAS_TOP];
		if (a)
			lwsl_notice("top: %d.%u\n", a->u.i.whole, a->u.i.frac);
		a = ps->css_pos[CCPAS_RIGHT];
		if (a)
			lwsl_notice("right: %d.%u\n", a->u.i.whole, a->u.i.frac);
		a = ps->css_pos[CCPAS_BOTTOM];
		if (a)
			lwsl_notice("bottom: %d.%u\n", a->u.i.whole, a->u.i.frac);
		a = ps->css_pos[CCPAS_LEFT];
		if (a)
			lwsl_notice("left: %d.%u\n", a->u.i.whole, a->u.i.frac);

		a = ps->css_margin[CCPAS_TOP];
		if (a)
			lwsl_notice("margin top: %d.%u\n", a->u.i.whole, a->u.i.frac);
		a = ps->css_margin[CCPAS_RIGHT];
		if (a)
			lwsl_notice("margin right: %d.%u\n", a->u.i.whole, a->u.i.frac);
		a = ps->css_margin[CCPAS_BOTTOM];
		if (a)
			lwsl_notice("margin bottom: %d.%u\n", a->u.i.whole, a->u.i.frac);
		a = ps->css_margin[CCPAS_LEFT];
		if (a)
			lwsl_notice("margin left: %d.%u\n", a->u.i.whole, a->u.i.frac);

		a = ps->css_padding[CCPAS_TOP];
		if (a)
			lwsl_notice("padding top: %d.%u\n", a->u.i.whole, a->u.i.frac);
		a = ps->css_padding[CCPAS_RIGHT];
		if (a)
			lwsl_notice("padding right: %d.%u\n", a->u.i.whole, a->u.i.frac);
		a = ps->css_padding[CCPAS_BOTTOM];
		if (a)
			lwsl_notice("padding bottom: %d.%u\n", a->u.i.whole, a->u.i.frac);
		a = ps->css_padding[CCPAS_LEFT];
		if (a)
			lwsl_notice("padding left: %d.%u\n", a->u.i.whole, a->u.i.frac);

		a = lws_css_cascade_get_prop_atr(ctx, LCSP_PROP_FONT_SIZE);
		if (a)
			lwsl_notice("font-size: %d.%u\n", a->u.i.whole, a->u.i.frac);

		a = lws_css_cascade_get_prop_atr(ctx, LCSP_PROP_FONT_FAMILY);
		if (a)
			lwsl_notice("font-family: %s\n", (const char *)&a[1]);
	}

	printf(" },\n");

  return 0;
}


int main(int argc, char *argv[])
{
    FILE *file;
    uint8_t *buf;
    uint32_t file_size;

    if (argc < 2) {
        fprintf(stderr, "usage: %s filename\n", argv[0]);
        return 1;
    }
    file = fopen(argv[1], "rb");
    if (file == NULL) {
        perror("open file failed");
        return 1;
    }
    fseek(file, 0, SEEK_END);
    file_size = ftell(file);
    rewind(file);
    buf = malloc(file_size+1);
    if (buf == NULL) {
        perror("malloc failed");
        fclose(file);
        return 1;
    }

    fread(buf, 1, file_size, file);
    lws_dl_rend_t drt;
    lhp_ctx_t ctx;
    memset(&ctx, 0, sizeof(ctx));

    drt.dl = &displaylist;
    drt.w = ic.wh_px[0].whole;
    drt.h = ic.wh_px[1].whole;

    if (lws_lhp_construct(&ctx, test_cb, &drt, &ic)) {
        return 0;
    }
    ctx.flags = LHP_FLAG_DOCUMENT_END;
    ctx.base_url = strdup("");

    const uint8_t *data = buf;
    lws_lhp_parse(&ctx, &data, (size_t *)&file_size);

  return 0;
}

Following is an output of ASAN.
vuln35.html is in vuln35.zip

$ ./harness ../crash/vuln35.html
AddressSanitizer:DEADLYSIGNAL
=================================================================
==31750==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x5581486ac27d bp 0x7ffe943dc950 sp 0x7ffe943dc930 T0)
==31750==The signal is caused by a READ memory access.
==31750==Hint: address points to the zero page.
    #0 0x5581486ac27d in lhp_set_dlo_padding_margin /home/vagrant/libwebsockets/for_build/lib/misc/dlo/dlo-lhp.c:222
    #1 0x5581486900d9 in lws_lhp_parse /home/vagrant/libwebsockets/for_build/lib/misc/lhp.c:1043
    #2 0x55814868b93f in main harness.c:162
    #3 0x7fc8a803ed8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #4 0x7fc8a803ee3f in __libc_start_main_impl ../csu/libc-start.c:392
    #5 0x55814868a2a4 in _start (/home/vagrant/libwebsockets/for_build/harness+0xc2a4)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/vagrant/libwebsockets/for_build/lib/misc/dlo/dlo-lhp.c:222 in lhp_set_dlo_padding_margin
==31750==ABORTING

It is caused by these line.
When css_margin is initialized in lws_css_cascade, if lws_css_cascade_get_prop_atr returns NULL, css_margin[n] is NULL.

lws_css_cascade(ctx);

ps->css_margin[CCPAS_TOP] = lws_css_cascade_get_prop_atr(ctx, LCSP_PROP_MARGIN_TOP);
ps->css_margin[CCPAS_RIGHT] = lws_css_cascade_get_prop_atr(ctx, LCSP_PROP_MARGIN_RIGHT);
ps->css_margin[CCPAS_BOTTOM] = lws_css_cascade_get_prop_atr(ctx, LCSP_PROP_MARGIN_BOTTOM);
ps->css_margin[CCPAS_LEFT] = lws_css_cascade_get_prop_atr(ctx, LCSP_PROP_MARGIN_LEFT);

Then, when calling lws_csp_px with ps→css_margin[0] as pointer argument inside lhp_set_dlo_padding_margin, null pointer dereference occurs

lhp_set_dlo_padding_margin(ps, ps->dlo);

dlo->margin[n] = *lws_csp_px(ps->css_margin[n], ps);

Ricerca Security, Inc.

lws-team added a commit that referenced this issue May 12, 2024
iwashiira on github #3140
found the html / css calculation could end up with NULL margin sizes.

This causes an assert if we get this unacceptable situation as far as
lws_csp_px()
lws-team added a commit that referenced this issue May 12, 2024
iwashiira on github #3140
found the html / css calculation could end up with NULL margin sizes.
@lws-team
Copy link
Member

Thanks a lot for your work testing on lhp /dlo.

I pushed a couple of patches on main for this, does it actually help?

@iwashiira
Copy link
Contributor Author

Yes, it is working well.
Thanks.

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