Re: [patch] reject http header without colon (:) in the header name

2024-05-13 Thread Ben Kallus
> nginx is about as popular as GWS, same reasoning might be considered. What I'm saying is exceptional about GWS is not its popularity. Of course, Nginx (and Apache) are similarly popular. I'm arguing that because GWS is by design a single-purpose web server that serves the interest of a single co

Re: [patch] reject http header without colon (:) in the header name

2024-05-13 Thread Ben Kallus
Okay; I should have been more specific. I meant that nginx is unique among *general-purpose* web servers. GWS is something of an special case; it also accepts requests with no Host header, and doesn't validate the version string (e.g., HTTP/1.9 is accepted). Google has opted into these st

Re: [patch] reject http header without colon (:) in the header name

2024-05-07 Thread Ben Kallus
Nginx is the only widely-used HTTP server that ignores invalid field-lines. This behavior makes it trivial to fingerprint. I never reported this in the past because I assumed Maxim wouldn't care about that sort of thing. Now that he's out of the picture, maybe others will see things differently?

Re: [PATCH] Enforce that CR precede LF in chunk lines

2024-02-14 Thread Ben Kallus
> Overall, I don't think there is a big difference here. All I can say is that the hardest part of pulling off that type of attack is guessing the length correctly. If you want to make that job marginally easier, that's fine by me :) > It won't, because "-C" is a non-portable flag provided by a D

Re: [PATCH] Enforce that CR precede LF in chunk lines

2024-01-25 Thread Ben Kallus
> Still, there is a robustness principle which allows applications > to parse requests with various deviations from the grammar. Whether this is principle is good is a matter of opinion. I tend to lean toward thinking that it is not (as you can probably tell) but reasonable minds will differ on th

[PATCH] Enforce that CR precede LF in chunk lines

2024-01-24 Thread Ben Kallus
The HTTP/1.1 standards allow the recognition of bare LF as a line terminator in some contexts. From RFC 9112 Section 2.2: > Although the line terminator for the start-line and fields is the sequence > CRLF, a recipient MAY recognize a single LF as a line terminator and ignore > any preceding CR.

Re: Core: Avoid memcpy from NULL

2024-01-23 Thread Ben Kallus
Hi Maxim, > As already pointed out previously, there are no known cases > when memcpy(p, NULL, 0) can result in miscompilation of nginx > code, ... If you think there are cases when the code can be > miscompiled in practice, and not theoretically, please share. There is no such thing as "miscomp

Re: [njs] Ignoring UndefinedBehaviorSanitizer warnings where appropriate.

2024-01-23 Thread Ben Kallus
> Casting NaN to integer is undefined behavior, > but it is fine in some cases where we do additional checks later. > For example: > int64_t i64 = njs_unsafe_cast_double_to_int64(num); > if (i64 == num) { > // num is integer > } This could be fine, but it's not guaranteed by the standard. For

Re: Core: Avoid memcpy from NULL

2024-01-09 Thread Ben Kallus
> This demonstrates that your patch > is clearly insufficient. Further, Vladimir's patch is clearly > insufficient too, as shown for the another patch in the same > patch series. "Insufficient" only when compared to a hypothetical perfectly exhaustive patch that requires "huge work," as you put i

[PATCH] Satisfy UBSan in njs

2024-01-03 Thread Ben Kallus
, my application no longer has any UBSan alerts. # HG changeset patch # User Ben Kallus # Date 1704329280 18000 # Wed Jan 03 19:48:00 2024 -0500 # Node ID 85d5846984fc2731ad74f91f21c74be67d6974a9 # Parent 4a15613f4e8bb4a8349ee1cefbae07585da4cbc6 Prevent undefined operations on NULL, INF, and

Re: Core: Avoid memcpy from NULL

2024-01-03 Thread Ben Kallus
in my configuration: # HG changeset patch # User Ben Kallus # Date 1704322684 18000 # Wed Jan 03 17:58:04 2024 -0500 # Node ID 04eb4b1622d1a488f14bb6d5af25e422ff23d82d # Parent ee40e2b1d0833b46128a357fbc84c6e23be9be07 Add check to ngx_pstrdup to prevent 0-length memcpy. diff -r

Re: Core: Avoid memcpy from NULL

2023-12-29 Thread Ben Kallus
> Still, -O0 is often used at least during development, and it might > be unreasonable to introduce extra function calls in basic > primitives. I don't think this is a major cause for concern. It is perfectly reasonable for ngx_memcpy be a wrapper function around memcpy; I think most people would

Re: Core: Avoid memcpy from NULL

2023-12-16 Thread Ben Kallus
> In general macro definitions in nginx are used everywhere for > efficiency reasons Clang inlines short functions with -O1, and GCC does so with -O2 or -O1 -finline-small-functions. Are there any platforms that Nginx needs to support for which short function inlining isn't sufficient to solve thi

Re: Core: Avoid memcpy from NULL

2023-12-15 Thread Ben Kallus
assert in `ngx_memcpy` > and `ngx_cpymem` would be fully enough, in my opinion. > > Regards, > Sergey. > > On 15.12.2023 03:41, Maxim Dounin wrote: > >> Hello! >> >> On Wed, Dec 13, 2023 at 11:09:28AM -0500, Ben Kallus wrote: >> >> Nginx exec

Core: Avoid memcpy from NULL

2023-12-13 Thread Ben Kallus
uch a change, a more straightforward and tidy solution was desired. It may also be worth considering adding checks for NULL memset, memmove, etc. I think this is not necessary unless it is demonstrated that Nginx actually executes such undefined calls. # HG changeset patch # User Ben Kallus # Date 1702406