FabianLars / tauri-plugin-oauth

Apache License 2.0
79 stars 20 forks source link

Statically allocating space for 16 headers is too few for redirects from some OAuth providers #21

Closed mrodz closed 2 months ago

mrodz commented 3 months ago

https://github.com/FabianLars/tauri-plugin-oauth/blob/50dadbf4a81cba51f625587b7722892f0b4316a6/src/lib.rs#L114

Google's OAuth API for web apps will send more than 16 headers, which causes this line:

https://github.com/FabianLars/tauri-plugin-oauth/blob/50dadbf4a81cba51f625587b7722892f0b4316a6/src/lib.rs#L116

To throw:

thread '<unnamed>' panicked at desktop\src\auth.rs:118:28:
called `Result::unwrap()` on an `Err` value: TooManyHeaders
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

For my use case.

Is this behavior present and noticeable for other users of this plugin?

mrodz commented 3 months ago

A quick note: I did not see any stack trace when originally using this plugin directly and was only met with an OAuth redirect to a: 127.0.0.1:PORT did not send any data. net::ERR_EMPTY_RESPONSE chromium page. I only found out this was the source of the bug by vendoring the dependency and manually doing some testing.

FabianLars commented 3 months ago

Is this behavior present and noticeable for other users of this plugin?

For what it's worth, you're the first one to tell me about this. What's weird is that google is exactly the reason i created this plugin so it's probably the most tested oauth provider - i guess something changed.

Do you have a rough estimate on the amount of headers?

mrodz commented 3 months ago

On a fresh Chrome profile, the redirect appears to send exactly 16 headers. However, some users may use browser extensions or have an application-level firewall that adds extra headers to HTTP requests, which is how I ended up with some in the 17-19 range and a TCP server crash without warning or message. I added space for 32 headers in my own app just to account for the case where there are extra headers.

Whether or not this is important enough to warrant additional space allocated in the handler function is up to you. I just wanted to bring this to your attention :)

Here are all of the incoming headers for one request:

Host=127.0.0.1:59778
Connection=keep-alive
Cache-Control=max-age=0
DNT=1
Upgrade-Insecure-Requests=1
User-Agent=******
Accept=text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7
Sec-Fetch-Site=cross-site
Sec-Fetch-Mode=navigate
Sec-Fetch-User=?1
Sec-Fetch-Dest=document
sec-ch-ua="Google Chrome";v="125", "Chromium";v="125", "Not.A/Brand";v="24"
sec-ch-ua-mobile=?0
sec-ch-ua-platform="Windows"
Referer=https://accounts.google.com/
Accept-Encoding=gzip, deflate, br, zstd
Accept-Language=en-US,en;q=0.9,es-US;q=0.8,es;q=0.7
Cookie=******
[desktop\src\auth.rs:118:5] header_c = 18