Skip to content

Fix 9 security vulnerabilities found in audit - #366

Merged
etr merged 2 commits into
masterfrom
feature/FEATURE-security-fixes
Feb 14, 2026
Merged

Fix 9 security vulnerabilities found in audit#366
etr merged 2 commits into
masterfrom
feature/FEATURE-security-fixes

Conversation

@etr

@etretr commented Feb 14, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes 9 security vulnerabilities identified in a security audit, ranging from HIGH to LOW severity:

  • [HIGH] Path traversal in file uploads — When generate_random_filename_on_upload is disabled, a crafted filename field (e.g. ../escape) could write files outside the upload directory. Added sanitize_upload_filename() that extracts the basename and rejects ., .., and empty names.
  • [MEDIUM] TOCTOU race in file_response — Replaced stat()-then-open() with open()-then-fstat() to eliminate the race window. Added O_NOFOLLOW on non-Windows to prevent symlink following.
  • [MEDIUM] FD leak on lseek failurefile_response::get_raw_response() leaked the fd when lseek() returned -1. Now closes fd before returning nullptr.
  • [MEDIUM] NULL pointer dereferenceMHD_get_connection_info() can return nullptr; added null check before dereferencing conninfo->connect_fd for TCP_NODELAY.
  • [MEDIUM] Uninitialized _file_sizefile_info::_file_size was uninitialized in the default constructor. Now zero-initialized.
  • [MEDIUM] Auth skip path bypassshould_skip_auth() now normalizes paths (resolves .. and . segments) before comparing, preventing bypass via /public/../protected.
  • [LOW] free() vs MHD_free() — Digest auth username was freed with free() instead of MHD_free(). Fixed to use the correct deallocator.
  • [LOW] Unchecked write error — File upload writes were not checked for errors. Added !good() check after write(), returning MHD_NO on failure.
  • [LOW] FD leak on zero-size file — Zero-size files took a code path that didn't close the fd. Now closes fd before creating an empty buffer response.

Test plan

  • All 14 existing tests pass (make check)
  • New test: path traversal filename (../escape) is rejected, no file created outside upload dir
  • New test: path-like filename (some/path/myfile.txt) is sanitized to basename only
  • New test: /public/../protected requires auth (not bypassed via .. traversal)

- Fix path traversal in file uploads: sanitize filenames to basename,
reject ".." and "." when generate_random_filename_on_upload is off
- Fix TOCTOU race in file_response: replace stat-then-open with
open-then-fstat; add O_NOFOLLOW on non-Windows
- Fix FD leaks in file_response: close fd on lseek failure and
zero-size file path
- Fix NULL deref in answer_to_connection: check conninfo before use
- Fix uninitialized _file_size in file_info (default to 0)
- Fix auth skip path bypass: normalize path (resolve ".." segments)
before comparing against skip paths
- Fix free() vs MHD_free() for digest auth username
- Fix unchecked write error during file upload
@codecov

codecovBot commented Feb 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 34.88372% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.93%. Comparing base (755ecc1) to head (28fa48f).
⚠️ Report is 3 commits behind head on master.

Files with missing linesPatch %Lines
src/webserver.cpp37.03%3 Missing and 14 partials ⚠️
src/file_response.cpp12.50%2 Missing and 5 partials ⚠️
src/http_utils.cpp42.85%1 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@ Coverage Diff @@## master #366 +/- ##
==========================================
- Coverage 68.70% 67.93% -0.78% 
==========================================
Files 28 28 Lines 1601 1634 +33 Branches 651 672 +21 ==========================================
+ Hits 1100 1110 +10 - Misses 58 64 +6 - Partials 443 460 +17 
Files with missing linesCoverage Δ
src/http_request.cpp60.41% <100.00%> (ø)
src/httpserver/file_info.hpp100.00% <ø> (ø)
src/httpserver/http_utils.hpp75.00% <ø> (ø)
src/http_utils.cpp66.03% <42.85%> (-0.80%)⬇️
src/file_response.cpp35.71% <12.50%> (-18.84%)⬇️
src/webserver.cpp56.33% <37.03%> (-0.98%)⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 755ecc1...28fa48f. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@etr
etr merged commit 1a26dfc into masterFeb 14, 2026
42 of 44 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@etr