Repository navigation
Security updates into master - #51
Merged
Merged
Conversation
…er & Host header path traversal
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security hardening, per-IP throttling fix, SSL unit tests, and an OpenSSL CI build with caching. 10 commits, 9 files changed, no files added or removed.
Summary
This branch closes four remotely reachable issues in the request and connection path, fixes an SSL build error, adds the first SSL unit tests, and stands up an OpenSSL build in CI so the
sslconfiguration is actually compiled and tested. It also includes small, behaviour-neutral refactors in the driver and accept path.Security fixes (remotely exploitable)
CGI input-file protocol injection. The per-request
.infile uses a line-basedTYPE=KEY=VALUEprotocol that the CGI side parses into trustedSERVER,POST,COOKIE, andFILESentries. Client-supplied POST field names and values (and the multipart filename and mime) were written into it without escaping, so a URL-decoded value containing a newline could inject additional protocol lines and forge variables such asREMOTE_ADDR,HTTPS, orSCRIPT_FILENAME. CR and LF are now stripped from these fields before they are written. Single-line values, which is everything that worked before, are unaffected.httpoxy (CVE-2016-5385 class). Every request header was mapped into the CGI environment as
HTTP_*, so aProxy:request header becameHTTP_PROXYand could redirect a script's outbound HTTP traffic. TheProxyheader is now skipped when building the environment. No legitimate client sends it.Host-header path traversal. The
Hostheader flows throughshorthost()into the filesystem root without hostname validation, so aHostcontaining..reached path construction. Routing now rejects a shorthost containing..or a null byte with the same response as an unknown domain. No valid hostname contains either.Per-IP connection cap was not enforced. The per-IP counter was keyed on
driver.ip, which returns0.0.0.0untilopenConnectionruns, while the admission check keyed on the real peer IP, so the check always read zero and the 32-per-IP limit never fired. A single IP could fill the 2048-slot global queue and starve other clients. The peer address is now set on the driver at accept time, and the count is incremented at enqueue under the same lock as the check, so queued plus active connections per real IP are bounded. This also closes the check-then-increment race.TLS / build fix
SSL_pendingconst mismatch.hasBuffered()isconst, so itssslpointer isconst(SSL*), which could not bind to the ImportC binding's mutablessl_st*parameter and broke thesslbuild. The call now passescast(SSL*) ssl. The underlying C function is genuinelyconst, so the cast asserts const-correctness the binding dropped rather than hiding anything. It is the onlyconstmethod that touches an SSL function, so no other call sites change.Tests
SSL unit tests added. The previously empty
ssl.dunittest now covers SNI context matching (findContext/hasCertificate), including a case that documents the currentendsWithsuffix match accepting a look-alike host, and exercisesgenerateKeyagainst libcrypto (2048-bit keygen, PEM written and checked). These run only underdub test --config=ssl.CI: build and cache OpenSSL
The
sslconfiguration was never compiled in CI becausedub testbuilds the default configuration. CI now:build-essentialandperl,./Configure linux-x86_64 no-teststhenmake -j),actions/cache@v5, keyed on the pinned OpenSSL submodule commit, so the build is skipped on a cache hit,--config=sslpass.Refactors (behaviour-neutral)
ipandportare resolved once when the peer address is set and returned from cached fields, instead of re-parsing the address on every call.accept()is a single ternary, withHTTPSaliased toHTTPin the non-SSL build so it compiles in both configurations without astatic ifor a manifest constant.secureis never true in a non-SSL build, so the alias is never constructed.