[PATCH] Satisfy UBSan in njs

[Dmitry Volyntsev]

On 1/3/24 4:55 PM, Ben Kallus wrote:
> When I run my nginx+njs application with UBSan enabled, I encounter a
> few instances of undefined behavior in njs:
>
> 1. A memcpy from NULL
> 2. A couple of offsets applied to NULL
> 3. A u32 assigned to nan
> 4. A u32 assigned to inf
>
> This patch adds checks to prevent these undefined operations. With it,
> my application no longer has any UBSan alerts.

Hi Ben,

I did a bunch of patches related to UBSan in njs core, most notably
https://hg.nginx.org/njs/rev/0490f1ae4cf5.
Now unit tests and test262 pass without warnings.

Thank you for prodding.

>
> # HG changeset patch
> # User Ben Kallus <benjamin.p.kallus.gr at dartmouth.edu>
> # Date 1704329280 18000
> #      Wed Jan 03 19:48:00 2024 -0500
> # Node ID 85d5846984fc2731ad74f91f21c74be67d6974a9
> # Parent  4a15613f4e8bb4a8349ee1cefbae07585da4cbc6
> Prevent undefined operations on NULL, INF, and NAN
>
> diff -r 4a15613f4e8b -r 85d5846984fc nginx/ngx_http_js_module.c
> --- a/nginx/ngx_http_js_module.c        Tue Dec 19 12:37:05 2023 -0800
> +++ b/nginx/ngx_http_js_module.c        Wed Jan 03 19:48:00 2024 -0500
> @@ -2717,7 +2717,9 @@
>
>       for ( /* void */ ; cl; cl = cl->next) {
>           buf = cl->buf;
> -        p = ngx_cpymem(p, buf->pos, buf->last - buf->pos);
> +        if (buf->last - buf->pos > 0) {
> +            p = ngx_cpymem(p, buf->pos, buf->last - buf->pos);
> +        }
>       }
>
>   done:
> diff -r 4a15613f4e8b -r 85d5846984fc src/njs_extern.c
> --- a/src/njs_extern.c  Tue Dec 19 12:37:05 2023 -0800
> +++ b/src/njs_extern.c  Wed Jan 03 19:48:00 2024 -0500
> @@ -38,7 +38,10 @@
>       lhq.proto = &njs_object_hash_proto;
>       lhq.pool = vm->mem_pool;
>
> -    end = external + n;
> +    end = external;
> +    if (n > 0) {
> +        end += n;
> +    }
>
>       while (external < end) {
>
> diff -r 4a15613f4e8b -r 85d5846984fc src/njs_number.h
> --- a/src/njs_number.h  Tue Dec 19 12:37:05 2023 -0800
> +++ b/src/njs_number.h  Wed Jan 03 19:48:00 2024 -0500
> @@ -41,6 +41,10 @@
>   {
>       uint32_t  u32;
>
> +    if (isnan(num) || isinf(num)) {
> +        return 0;
> +    }
> +
>       u32 = num;
>
>       return (u32 == num && u32 != 0xffffffff);
> diff -r 4a15613f4e8b -r 85d5846984fc src/njs_object.c
> --- a/src/njs_object.c  Tue Dec 19 12:37:05 2023 -0800
> +++ b/src/njs_object.c  Wed Jan 03 19:48:00 2024 -0500
> @@ -598,7 +598,10 @@
>       start = array->start;
>
>       p = start;
> -    end = p + array->length;
> +    end = p;
> +    if (array->length > 0) {
> +        end += array->length;
> +    }
>
>       switch (kind) {
>       case NJS_ENUM_KEYS:
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel