Skip to content

Commit c26c373

Browse files
etrclaude
etr
and
claude
committed
ci: fix valgrind UAF root cause, CodeQL overflow, Windows symlink check
The previous commit (87185ea) added wildcard valgrind suppressions for per-route firing paths under the rationale that the read was a "false positive." It was not. `test_utils::as_shared(stack_resource)` wrapped stack-allocated http_resources in a shared_ptr with a no-op deleter; the webserver kept those shared_ptrs in its route_table_, but the underlying storage died when the test body returned. MHD's request_completed callback fires from a daemon worker thread during `ws->stop()` (called in tear_down, AFTER the test body's locals have already destructed), so the per-route firing path dereferenced freed stack memory through `mr->resource_weak_.lock()`. The "Conditional jump on uninitialised value" valgrind report was a real UAF, not a control-block read race. Fix: migrate every `as_shared(stack_obj)` call site (226 across 11 files) to `std::make_shared<T>(...)`. With heap-owned resources the lifetime model becomes correct -- the webserver's shared_ptr keeps the resource alive until `~webserver_impl` runs, which is after `stop()` has drained MHD's workers. The `as_shared` helper is removed from test_utils.hpp; the file now carries a note documenting why it was deleted. The two wildcard suppressions in test/libhttpserver.supp are also removed; only the legitimate `gnutls_session_get_data` Memcheck:Cond suppression remains. Also fixed on this branch: * src/detail/ip_representation.cpp: CodeQL flagged `(16 - i) * a.pieces[i]` as an int*int -> int64_t overflow path. Cast the (16-i) factor to int64_t so the multiply is performed in the destination type. * src/Makefile.am install-data-hook: `ln -s` on MSYS2/mingw silently falls back to a file copy (sometimes failing entirely without admin rights). Wrap the symlink creation in `{ ln -s ... || cp ...; }` so the alias is always installed under one form or another. * Makefile.am check-install-layout: relax `test -L httpserverpp` to `test -e httpserverpp`. Both a POSIX symlink and an MSYS2 file copy satisfy the "include resolves to umbrella" property the hook is establishing. The file-size ceiling temp-bump from 87185ea is left in place; walking the nine offenders back below 500 LOC is the documented follow-up and is independent of this fix. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 87185ea commit c26c373

15 files changed

Lines changed: 517 additions & 557 deletions

‎Makefile.am‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,12 @@ check-install-layout:
209209
if test "$(CHECK_INSTALL_SHARED)" != "yes"; then rm -rf $(CHECK_INSTALL_STAGE); fi; \
210210
exit 1; \
211211
fi
212-
@if ! test -L $(CHECK_INSTALL_STAGE)$(includedir)/httpserverpp; then \
213-
echo "FAIL: httpserverpp symlink not installed under $(includedir)"; \
212+
@# Accept either a symlink (POSIX) or a regular file (MSYS2/mingw, where
213+
@# `ln -s` falls back to a file copy unless MSYS=winsymlinks is set).
214+
@# Either form satisfies the "includes #include <httpserverpp> resolve to
215+
@# the umbrella" property the install hook is establishing.
216+
@if ! test -e $(CHECK_INSTALL_STAGE)$(includedir)/httpserverpp; then \
217+
echo "FAIL: httpserverpp alias not installed under $(includedir)"; \
214218
if test "$(CHECK_INSTALL_SHARED)" != "yes"; then rm -rf $(CHECK_INSTALL_STAGE); fi; \
215219
exit 1; \
216220
fi

‎src/Makefile.am‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,16 @@ libhttpserver_la_CXXFLAGS = $(AM_CXXFLAGS)
5454
libhttpserver_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined -version-number @LHT_LDF_VERSION@
5555

5656
install-data-hook:
57-
(mkdir -p $(DESTDIR)$(includedir) && cd $(DESTDIR)$(includedir) && $(LN_S) -f httpserver.hpp httpserverpp)
57+
@# Install an `<httpserverpp>` alias next to `<httpserver.hpp>`. On POSIX
58+
@# this is a symlink. On MSYS2/mingw `ln -s` typically falls back to a
59+
@# file copy (it produces a regular file, not a link); when even that
60+
@# fails (e.g. tightened sandboxes, or `ln` missing the `-s` flag in
61+
@# pristine mingw shells) we fall back to an explicit `cp`. The
62+
@# downstream `check-install-layout` rule accepts either form.
63+
mkdir -p $(DESTDIR)$(includedir)
64+
cd $(DESTDIR)$(includedir) && \
65+
rm -f httpserverpp && \
66+
{ $(LN_S) httpserver.hpp httpserverpp || cp httpserver.hpp httpserverpp; }
5867

5968
uninstall-hook:
6069
(cd $(DESTDIR)$(includedir) && rm -f httpserverpp)

‎src/detail/ip_representation.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,12 @@ void accumulate_octet_score(const ip_representation& a,
268268
const ip_representation& b, int i,
269269
int64_t& a_score, int64_t& b_score) {
270270
if (!(CHECK_BIT(a.mask, i) && CHECK_BIT(b.mask, i))) return;
271-
a_score += (16 - i) * a.pieces[i];
272-
b_score += (16 - i) * b.pieces[i];
271+
// Cast the (16 - i) factor to int64_t before the multiply so the product
272+
// is computed in the destination type. Without the cast the multiplication
273+
// happens in int and only the result is widened, which CodeQL flags as a
274+
// potential overflow.
275+
a_score += static_cast<int64_t>(16 - i) * a.pieces[i];
276+
b_score += static_cast<int64_t>(16 - i) * b.pieces[i];
273277
}
274278

275279
// True when both v4-mapped-prefix octets on a and b are 0x00 or 0xFF.

‎test/integ/authentication.cpp‎

Lines changed: 56 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ LT_END_SUITE(authentication_suite)
125125
LT_BEGIN_AUTO_TEST(authentication_suite, base_auth)
126126
webserver ws{create_webserver(PORT)};
127127

128-
user_pass_resource user_pass;
129-
ws.register_path("base", as_shared(user_pass));
128+
auto user_pass = std::make_shared<user_pass_resource>();
129+
ws.register_path("base", user_pass);
130130
ws.start(false);
131131

132132
curl_global_init(CURL_GLOBAL_ALL);
@@ -150,8 +150,8 @@ LT_END_AUTO_TEST(base_auth)
150150
LT_BEGIN_AUTO_TEST(authentication_suite, base_auth_fail)
151151
webserver ws{create_webserver(PORT)};
152152

153-
user_pass_resource user_pass;
154-
ws.register_path("base", as_shared(user_pass));
153+
auto user_pass = std::make_shared<user_pass_resource>();
154+
ws.register_path("base", user_pass);
155155
ws.start(false);
156156

157157
curl_global_init(CURL_GLOBAL_ALL);
@@ -247,8 +247,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth)
247247
.digest_auth_random("myrandom")
248248
.nonce_nc_size(300)};
249249

250-
digest_resource digest;
251-
ws.register_path("base", as_shared(digest));
250+
auto digest = std::make_shared<digest_resource>();
251+
ws.register_path("base", digest);
252252
ws.start(false);
253253

254254
#if defined(_WINDOWS)
@@ -294,8 +294,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_wrong_pass)
294294
.digest_auth_random("myrandom")
295295
.nonce_nc_size(300)};
296296

297-
digest_resource digest;
298-
ws.register_path("base", as_shared(digest));
297+
auto digest = std::make_shared<digest_resource>();
298+
ws.register_path("base", digest);
299299
ws.start(false);
300300

301301
#if defined(_WINDOWS)
@@ -339,8 +339,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5)
339339
.digest_auth_random("myrandom")
340340
.nonce_nc_size(300)};
341341

342-
digest_ha1_md5_resource digest_ha1;
343-
ws.register_path("base", as_shared(digest_ha1));
342+
auto digest_ha1 = std::make_shared<digest_ha1_md5_resource>();
343+
ws.register_path("base", digest_ha1);
344344
ws.start(false);
345345

346346
#if defined(_WINDOWS)
@@ -386,8 +386,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5_wrong_pass)
386386
.digest_auth_random("myrandom")
387387
.nonce_nc_size(300)};
388388

389-
digest_ha1_md5_resource digest_ha1;
390-
ws.register_path("base", as_shared(digest_ha1));
389+
auto digest_ha1 = std::make_shared<digest_ha1_md5_resource>();
390+
ws.register_path("base", digest_ha1);
391391
ws.start(false);
392392

393393
#if defined(_WINDOWS)
@@ -431,8 +431,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_sha256)
431431
.digest_auth_random("myrandom")
432432
.nonce_nc_size(300)};
433433

434-
digest_ha1_sha256_resource digest_ha1;
435-
ws.register_path("base", as_shared(digest_ha1));
434+
auto digest_ha1 = std::make_shared<digest_ha1_sha256_resource>();
435+
ws.register_path("base", digest_ha1);
436436
ws.start(false);
437437

438438
#if defined(_WINDOWS)
@@ -478,8 +478,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_sha256_wrong_pass)
478478
.digest_auth_random("myrandom")
479479
.nonce_nc_size(300)};
480480

481-
digest_ha1_sha256_resource digest_ha1;
482-
ws.register_path("base", as_shared(digest_ha1));
481+
auto digest_ha1 = std::make_shared<digest_ha1_sha256_resource>();
482+
ws.register_path("base", digest_ha1);
483483
ws.start(false);
484484

485485
#if defined(_WINDOWS)
@@ -550,8 +550,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_user_cache_no_auth)
550550
.digest_auth_random("myrandom")
551551
.nonce_nc_size(300)};
552552

553-
digest_user_cache_resource resource;
554-
ws.register_path("cache_test", as_shared(resource));
553+
auto resource = std::make_shared<digest_user_cache_resource>();
554+
ws.register_path("cache_test", resource);
555555
ws.start(false);
556556

557557
curl_global_init(CURL_GLOBAL_ALL);
@@ -581,8 +581,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_user_cache_with_auth)
581581
.digest_auth_random("myrandom")
582582
.nonce_nc_size(300)};
583583

584-
digest_user_cache_resource resource;
585-
ws.register_path("cache_test", as_shared(resource));
584+
auto resource = std::make_shared<digest_user_cache_resource>();
585+
ws.register_path("cache_test", resource);
586586
ws.start(false);
587587

588588
curl_global_init(CURL_GLOBAL_ALL);
@@ -637,8 +637,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, centralized_auth_fail)
637637
webserver ws{create_webserver(PORT)
638638
.auth_handler(centralized_auth_handler)};
639639

640-
simple_resource sr;
641-
ws.register_path("protected", as_shared(sr));
640+
auto sr = std::make_shared<simple_resource>();
641+
ws.register_path("protected", sr);
642642
ws.start(false);
643643

644644
curl_global_init(CURL_GLOBAL_ALL);
@@ -664,8 +664,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, centralized_auth_success)
664664
webserver ws{create_webserver(PORT)
665665
.auth_handler(centralized_auth_handler)};
666666

667-
simple_resource sr;
668-
ws.register_path("protected", as_shared(sr));
667+
auto sr = std::make_shared<simple_resource>();
668+
ws.register_path("protected", sr);
669669
ws.start(false);
670670

671671
curl_global_init(CURL_GLOBAL_ALL);
@@ -694,10 +694,10 @@ LT_BEGIN_AUTO_TEST(authentication_suite, auth_skip_paths)
694694
.auth_handler(centralized_auth_handler)
695695
.auth_skip_paths({"/health", "/public/*"})};
696696

697-
simple_resource sr;
698-
ws.register_path("health", as_shared(sr));
699-
ws.register_path("public/info", as_shared(sr));
700-
ws.register_path("protected", as_shared(sr));
697+
auto sr = std::make_shared<simple_resource>();
698+
ws.register_path("health", sr);
699+
ws.register_path("public/info", sr);
700+
ws.register_path("protected", sr);
701701
ws.start(false);
702702

703703
curl_global_init(CURL_GLOBAL_ALL);
@@ -759,8 +759,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, auth_skip_paths_no_partial_match)
759759
.auth_handler(centralized_auth_handler)
760760
.auth_skip_paths({"/public/*"})};
761761

762-
simple_resource sr;
763-
ws.register_path("publicinfo", as_shared(sr));
762+
auto sr = std::make_shared<simple_resource>();
763+
ws.register_path("publicinfo", sr);
764764
ws.start(false);
765765

766766
curl_global_init(CURL_GLOBAL_ALL);
@@ -789,8 +789,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, auth_skip_paths_deep_nested)
789789
.auth_handler(centralized_auth_handler)
790790
.auth_skip_paths({"/api/v1/public/*"})};
791791

792-
simple_resource sr;
793-
ws.register_path("api/v1/public/users/list", as_shared(sr));
792+
auto sr = std::make_shared<simple_resource>();
793+
ws.register_path("api/v1/public/users/list", sr);
794794
ws.start(false);
795795

796796
curl_global_init(CURL_GLOBAL_ALL);
@@ -826,8 +826,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, centralized_auth_post_method)
826826
webserver ws{create_webserver(PORT)
827827
.auth_handler(centralized_auth_handler)};
828828

829-
post_resource pr;
830-
ws.register_path("data", as_shared(pr));
829+
auto pr = std::make_shared<post_resource>();
830+
ws.register_path("data", pr);
831831
ws.start(false);
832832

833833
curl_global_init(CURL_GLOBAL_ALL);
@@ -874,8 +874,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, centralized_auth_wrong_credentials)
874874
webserver ws{create_webserver(PORT)
875875
.auth_handler(centralized_auth_handler)};
876876

877-
simple_resource sr;
878-
ws.register_path("protected", as_shared(sr));
877+
auto sr = std::make_shared<simple_resource>();
878+
ws.register_path("protected", sr);
879879
ws.start(false);
880880

881881
curl_global_init(CURL_GLOBAL_ALL);
@@ -921,8 +921,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, centralized_auth_not_found)
921921
webserver ws{create_webserver(PORT)
922922
.auth_handler(centralized_auth_handler)};
923923

924-
simple_resource sr;
925-
ws.register_path("exists", as_shared(sr));
924+
auto sr = std::make_shared<simple_resource>();
925+
ws.register_path("exists", sr);
926926
ws.start(false);
927927

928928
curl_global_init(CURL_GLOBAL_ALL);
@@ -950,8 +950,8 @@ LT_END_AUTO_TEST(centralized_auth_not_found)
950950
LT_BEGIN_AUTO_TEST(authentication_suite, no_auth_handler_default)
951951
webserver ws{create_webserver(PORT)}; // No auth_handler
952952

953-
simple_resource sr;
954-
ws.register_path("open", as_shared(sr));
953+
auto sr = std::make_shared<simple_resource>();
954+
ws.register_path("open", sr);
955955
ws.start(false);
956956

957957
curl_global_init(CURL_GLOBAL_ALL);
@@ -981,11 +981,11 @@ LT_BEGIN_AUTO_TEST(authentication_suite, auth_multiple_skip_paths)
981981
.auth_handler(centralized_auth_handler)
982982
.auth_skip_paths({"/health", "/metrics", "/status", "/public/*"})};
983983

984-
simple_resource sr;
985-
ws.register_path("health", as_shared(sr));
986-
ws.register_path("metrics", as_shared(sr));
987-
ws.register_path("status", as_shared(sr));
988-
ws.register_path("protected", as_shared(sr));
984+
auto sr = std::make_shared<simple_resource>();
985+
ws.register_path("health", sr);
986+
ws.register_path("metrics", sr);
987+
ws.register_path("status", sr);
988+
ws.register_path("protected", sr);
989989
ws.start(false);
990990

991991
curl_global_init(CURL_GLOBAL_ALL);
@@ -1059,8 +1059,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, auth_skip_path_root)
10591059
.auth_handler(centralized_auth_handler)
10601060
.auth_skip_paths({"/"})};
10611061

1062-
simple_resource sr;
1063-
ws.register_prefix("/", as_shared(sr));
1062+
auto sr = std::make_shared<simple_resource>();
1063+
ws.register_prefix("/", sr);
10641064
ws.start(false);
10651065

10661066
curl_global_init(CURL_GLOBAL_ALL);
@@ -1090,10 +1090,10 @@ LT_BEGIN_AUTO_TEST(authentication_suite, auth_skip_path_wildcard)
10901090
.auth_handler(centralized_auth_handler)
10911091
.auth_skip_paths({"/pub/*"})};
10921092

1093-
simple_resource sr;
1094-
ws.register_path("pub/anything", as_shared(sr));
1095-
ws.register_path("pub/nested/path", as_shared(sr));
1096-
ws.register_path("private/secret", as_shared(sr));
1093+
auto sr = std::make_shared<simple_resource>();
1094+
ws.register_path("pub/anything", sr);
1095+
ws.register_path("pub/nested/path", sr);
1096+
ws.register_path("private/secret", sr);
10971097
ws.start(false);
10981098

10991099
curl_global_init(CURL_GLOBAL_ALL);
@@ -1152,8 +1152,8 @@ LT_BEGIN_AUTO_TEST(authentication_suite, auth_empty_skip_paths)
11521152
.auth_handler(centralized_auth_handler)
11531153
.auth_skip_paths({})}; // Empty skip paths
11541154

1155-
simple_resource sr;
1156-
ws.register_path("test", as_shared(sr));
1155+
auto sr = std::make_shared<simple_resource>();
1156+
ws.register_path("test", sr);
11571157
ws.start(false);
11581158

11591159
curl_global_init(CURL_GLOBAL_ALL);
@@ -1183,9 +1183,9 @@ LT_BEGIN_AUTO_TEST(authentication_suite, auth_skip_path_traversal_bypass)
11831183
.auth_handler(centralized_auth_handler)
11841184
.auth_skip_paths({"/public/*"})};
11851185

1186-
simple_resource sr;
1187-
ws.register_path("protected", as_shared(sr));
1188-
ws.register_path("public/info", as_shared(sr));
1186+
auto sr = std::make_shared<simple_resource>();
1187+
ws.register_path("protected", sr);
1188+
ws.register_path("public/info", sr);
11891189
ws.start(false);
11901190

11911191
curl_global_init(CURL_GLOBAL_ALL);

‎test/integ/ban_system.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ LT_BEGIN_AUTO_TEST(ban_system_suite, accept_default_block_blocks)
8080
webserver ws{create_webserver(PORT).default_policy(http_utils::ACCEPT)};
8181
ws.start(false);
8282

83-
ok_resource resource;
84-
ws.register_path("base", as_shared(resource));
83+
auto resource = std::make_shared<ok_resource>();
84+
ws.register_path("base", resource);
8585

8686
curl_global_init(CURL_GLOBAL_ALL);
8787

@@ -138,8 +138,8 @@ LT_BEGIN_AUTO_TEST(ban_system_suite, reject_policy_neither_allowed_nor_blocked)
138138
webserver ws{create_webserver(PORT).default_policy(http_utils::REJECT)};
139139
ws.start(false);
140140

141-
ok_resource resource;
142-
ws.register_path("base", as_shared(resource));
141+
auto resource = std::make_shared<ok_resource>();
142+
ws.register_path("base", resource);
143143

144144
curl_global_init(CURL_GLOBAL_ALL);
145145

@@ -164,8 +164,8 @@ LT_BEGIN_AUTO_TEST(ban_system_suite, block_with_weight_comparison)
164164
webserver ws{create_webserver(PORT).default_policy(http_utils::ACCEPT)};
165165
ws.start(false);
166166

167-
ok_resource resource;
168-
ws.register_path("base", as_shared(resource));
167+
auto resource = std::make_shared<ok_resource>();
168+
ws.register_path("base", resource);
169169

170170
curl_global_init(CURL_GLOBAL_ALL);
171171

@@ -198,8 +198,8 @@ LT_BEGIN_AUTO_TEST(ban_system_suite, block_specific_then_wildcard)
198198
webserver ws{create_webserver(PORT).default_policy(http_utils::ACCEPT)};
199199
ws.start(false);
200200

201-
ok_resource resource;
202-
ws.register_path("base", as_shared(resource));
201+
auto resource = std::make_shared<ok_resource>();
202+
ws.register_path("base", resource);
203203

204204
curl_global_init(CURL_GLOBAL_ALL);
205205

0 commit comments

Comments
 (0)