Crash libCurl 7.76.1 32-bit build in H3 transfers

[Dmitry Karpov via curl-library]

Hello,

I stepped on a crash in libCurl 7.76.1 32-bit in H3 transfers (MSVC, Win32).
It occurred on any direct H3 transfers from public H3 servers referred on Curl web page (https://bagder.github.io/HTTP3-test/)

I debugged the issue and it seems to be related to 32-bit builds and not Windows/MSVC-specific.

Here is the issue.
The file \lib\vquic\ngtcp2.c, line 605 in libCurl declares "cb_acked_stream_data_offset" function as follows:

static int
cb_acked_stream_data_offset(ngtcp2_conn *tconn, int64_t stream_id,
uint64_t offset, size_t datalen, void *user_data,
void *stream_user_data)

But the ngtcp2 library declares the callback slightly differently:

typedef int (*ngtcp2_acked_stream_data_offset)(
ngtcp2_conn *conn, int64_t stream_id, uint64_t offset, uint64_t datalen, void *user_data, void *stream_user_data);

So "size_t datalen" in libCurl is different from "uint64_t datalen" in the ngtcp2 library.

This is not an issue for 64-bit builds because size_t is equal to uint64_t, but for 32-bit builds it creates tricky problems because size_t is uint32_t for such builds,
and libCurl crashes because the cb_acked_stream_data_offset() function gets wrong pointers from 'user_data' and 'stream_user_data' params.

The obvious type correction seemed to fix the issue, and I was able to get H3 transfers working (although I didn't go beyond simple experiments).

The crash was so obvious, so it was a bit surprising how it was able to escape pre-release testing.
Maybe H3, as experimental feature, wasn't tested enough on 32-builds?
32-bit builds are still widely used on embedded devices, so it will help a lot if you guys look at this issue and look for other potential pitfalls like this when switching from 64-bit builds to 32-bit builds.

Thanks,
Dmitry Karpov

Roku Software Engineer

[Daniel Stenberg via curl-library]

Post by Dmitry Karpov via curl-library
So "size_t datalen" in libCurl is different from "uint64_t datalen" in the ngtcp2 library.

Fix pending: https://github.com/curl/curl/pull/7027

Post by Dmitry Karpov via curl-library
The crash was so obvious, so it was a bit surprising how it was able to
escape pre-release testing. Maybe H3, as experimental feature, wasn't tested
enough on 32-builds?

We don't test h3 at all yet on any platform, I'm sorry to say! :-O

We still lack a test server/proxy for this purpose and we have at least three
rather nasty-sounding HTTP/3 related bugs in KNOWN_BUGS:
https://curl.se/docs/knownbugs.html#If_the_HTTP_3_server_closes_conn

I would prefer doing h3 tests with a h3 <=> h1 proxy front, a little like
stunnel but for h3, so that we could still serve the core stuff with our
existing h1 test server. But it's not a hard requirement.

I would love to get this going better and further but I've not had time for it
lately and no one has stepped up wanting to sponsor my work on h3.

Post by Dmitry Karpov via curl-library
32-bit builds are still widely used on embedded devices, so it will help a
lot if you guys look at this issue and look for other potential pitfalls
like this when switching from 64-bit builds to 32-bit builds.

Once we get a few h3-tests landed, I think it would make sense and be good to
have at least one set of them running on a 32 bit system, yes. The availablity
of convenient CI builds on 32 bit systems might be a bit limited though.