Skip to content

url: fix use of an uninitialized variable - #13399

Closed
jimmy-park wants to merge 1 commit into
curl:masterfrom
jimmy-park:fix-ub
Closed

url: fix use of an uninitialized variable#13399
jimmy-park wants to merge 1 commit into
curl:masterfrom
jimmy-park:fix-ub

Conversation

@jimmy-park

Copy link
Copy Markdown
Contributor

The following bug is detected by UndefinedBehaviorSanitizer on Apple Clang.

/Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/url.c:810:10: runtime error: load of value 52, which is not a valid value for type 'bool'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/url.c:810:10 in

__ubsan_on_report (@__ubsan_on_report:3)
__ubsan::UndefinedBehaviorReport::UndefinedBehaviorReport(char const*, __ubsan::Location&, __sanitizer::InternalScopedString&) (@__ubsan::UndefinedBehaviorReport::UndefinedBehaviorReport(char const*, __ubsan::Location&, __sanitizer::InternalScopedString&):47)
__ubsan::Diag::~Diag() (@__ubsan::Diag::~Diag():64)
handleLoadInvalidValue(__ubsan::InvalidValueData*, unsigned long, __ubsan::ReportOptions) (@handleLoadInvalidValue(__ubsan::InvalidValueData*, unsigned long, __ubsan::ReportOptions):74)
__ubsan_handle_load_invalid_value (@__ubsan_handle_load_invalid_value:13)
extract_if_dead (/Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/url.c:810)
call_extract_if_dead (/Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/url.c:847)
Curl_conncache_foreach (/Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/conncache.c:332)
prune_dead_connections (/Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/url.c:876)
create_conn (/Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/url.c:3615)
Curl_connect (/Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/url.c:3846)
multi_runsingle (/Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/multi.c:1930)
curl_multi_perform (/Users/jimmy.park/Repos/curl_client/build/.cache/curl/760adaf446db44888b0a6d906e318c6147c808ba/lib/multi.c:2704)
http::CurlClient::RequestThread() (/Users/jimmy.park/Repos/curl_client/src/core/http/curl_client.cpp:374)
decltype(*std::declval<http::CurlClient*>().*std::declval<void (http::CurlClient::*)()>()()) std::__1::__invoke[abi:ue170006]<void (http::CurlClient::*)(), http::CurlClient*, void>(void (http::CurlClient::*&&)(), http::CurlClient*&&) (/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/__type_traits/invoke.h:308)
void std::__1::__thread_execute[abi:ue170006]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (http::CurlClient::*)(), http::CurlClient*, 2ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (http::CurlClient::*)(), http::CurlClient*>&, std::__1::__tuple_indices<2ul>) (/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/__thread/thread.h:227)
void* std::__1::__thread_proxy[abi:ue170006]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (http::CurlClient::*)(), http::CurlClient*>>(void*) (/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/__thread/thread.h:238)
_pthread_start (@_pthread_start:37)

input_pending could remain uninitialized if Curl_conn_is_alive() fails before assigning a value.
In my case, cf->conn->bits.close was 1 which caused a short-circuit.

curl/lib/cfilters.c

Lines 644 to 650 in b879ede

bool Curl_conn_is_alive(struct Curl_easy *data, struct connectdata *conn,
bool *input_pending)
{
struct Curl_cfilter *cf = conn->cfilter[FIRSTSOCKET];
return cf && !cf->conn->bits.close &&
cf->cft->is_alive(cf, data, input_pending);
}

@bagder bagder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@bagder

bagder commented Apr 17, 2024

Copy link
Copy Markdown
Member

the CI failures with ngtcp2 warnings are fixed with this: #13401

@bagder bagder closed this in 5fb0184 Apr 17, 2024
@bagder

bagder commented Apr 17, 2024

Copy link
Copy Markdown
Member

Thanks!

@jimmy-park
jimmy-park deleted the fix-ub branch April 18, 2024 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants