From e792429b506cea57c9f0f078440a26a5f1e6f9b5 Mon Sep 17 00:00:00 2001 From: whaffman Date: Fri, 31 Oct 2025 15:10:00 +0100 Subject: [PATCH] fix: add error handling for null bytes in config files and enforce location path rules --- webserv/config/ConfigManager.cpp | 6 +++++- webserv/config/GlobalConfig.cpp | 2 +- webserv/config/ServerConfig.cpp | 6 ++++++ webserv/config/directive/DirectiveFactory.cpp | 2 ++ webserv/config/directive/IntDirective.cpp | 18 +++++++++++++++--- webserv/config/directive/SizeDirective.cpp | 5 +++++ webserv/config/directive/StringDirective.cpp | 4 ++++ webserv/config/directive/VectorDirective.cpp | 4 ++++ webserv/http/HttpRequest.cpp | 19 +++++++++++++++++++ 9 files changed, 61 insertions(+), 5 deletions(-) diff --git a/webserv/config/ConfigManager.cpp b/webserv/config/ConfigManager.cpp index ac91db6..b4b58d2 100644 --- a/webserv/config/ConfigManager.cpp +++ b/webserv/config/ConfigManager.cpp @@ -55,8 +55,12 @@ void ConfigManager::parseConfigFile(const std::string &filePath) { throw std::runtime_error("Config file is empty: " + filePath); } - + utils::removeComments(content); + if (content.find('\0') != std::string::npos) + { + throw std::runtime_error("null byte detected in config file: " + filePath); + } globalConfig_ = std::make_unique(content); // Implement this function to handle global config diff --git a/webserv/config/GlobalConfig.cpp b/webserv/config/GlobalConfig.cpp index 05355f1..66ec3d2 100644 --- a/webserv/config/GlobalConfig.cpp +++ b/webserv/config/GlobalConfig.cpp @@ -50,7 +50,7 @@ void GlobalConfig::parseBlock(const std::string &block) pos = closeBrace + 1; } - if (block.find("location", 0) != std::string::npos) + if (directives.find("location", 0) != std::string::npos) { throw std::runtime_error("Location blocks are not allowed in the global context."); } diff --git a/webserv/config/ServerConfig.cpp b/webserv/config/ServerConfig.cpp index e91a79c..e7e74ee 100644 --- a/webserv/config/ServerConfig.cpp +++ b/webserv/config/ServerConfig.cpp @@ -48,6 +48,12 @@ void ServerConfig::parseBlock(const std::string &block) std::string locationPath = utils::trim(block.substr(locationPos + 9, bracePos - (locationPos + 9))); // TODO magic numbers // Add global declarations before this server block + + if (locationPath.front() != '/') + { + throw std::runtime_error("Location path must start with '/': " + locationPath); + } + directives += block.substr(pos, locationPos - pos); size_t closeBrace = utils::findCorrespondingClosingBrace(block, bracePos); if (closeBrace == std::string::npos) diff --git a/webserv/config/directive/DirectiveFactory.cpp b/webserv/config/directive/DirectiveFactory.cpp index 042bcbe..62781c9 100644 --- a/webserv/config/directive/DirectiveFactory.cpp +++ b/webserv/config/directive/DirectiveFactory.cpp @@ -37,6 +37,8 @@ std::unique_ptr DirectiveFactory::createDirective(const std::string { throw std::invalid_argument("Directive argument is empty: " + name); } + + if (type.empty()) { throw std::invalid_argument("Unsupported directive: " + name); diff --git a/webserv/config/directive/IntDirective.cpp b/webserv/config/directive/IntDirective.cpp index 1dcee92..ca3edd3 100644 --- a/webserv/config/directive/IntDirective.cpp +++ b/webserv/config/directive/IntDirective.cpp @@ -1,7 +1,7 @@ -#include // for IntDirective - #include // for ADirective #include // for DirectiveValueType +#include // for IntDirective +#include // for Log IntDirective::IntDirective(const std::string &name, const std::string &value) : ADirective(name) // NOLINT(bugprone-easily-swappable-parameters) @@ -11,7 +11,19 @@ IntDirective::IntDirective(const std::string &name, const std::string &value) void IntDirective::parse(const std::string &value) { - value_ = std::stoi(value); // TODO: check parsing + try + { + value_ = std::stoi(value); + Log::debug("IntDirective: parsed integer value " + std::to_string(value_) + " from string \"" + value + "\""); + } + catch (const std::invalid_argument &e) + { + throw std::invalid_argument("numeric value expected"); + } + catch (const std::out_of_range &e) + { + throw std::invalid_argument("IntDirective: integer out of range"); + } } DirectiveValueType IntDirective::getValueType() const diff --git a/webserv/config/directive/SizeDirective.cpp b/webserv/config/directive/SizeDirective.cpp index ed85c02..8bd8287 100644 --- a/webserv/config/directive/SizeDirective.cpp +++ b/webserv/config/directive/SizeDirective.cpp @@ -57,6 +57,11 @@ void SizeDirective::parse(const std::string &value) } value_ *= multiplier; + + if (value_ > 1000000000UL) // 1 GB limit for sanity should be a constant + { + throw std::invalid_argument("Size directive too large: " + value + " in " + name_); + } } DirectiveValueType SizeDirective::getValueType() const diff --git a/webserv/config/directive/StringDirective.cpp b/webserv/config/directive/StringDirective.cpp index f25e519..a35e027 100644 --- a/webserv/config/directive/StringDirective.cpp +++ b/webserv/config/directive/StringDirective.cpp @@ -11,6 +11,10 @@ StringDirective::StringDirective(const std::string &name, const std::string &val void StringDirective::parse(const std::string &value) { + if (value.size() > 4096) //TODO: use PATH_MAX or NAME_MAX where appropriate + { + throw std::invalid_argument("StringDirective: string value exceeds maximum length"); + } value_ = value; } diff --git a/webserv/config/directive/VectorDirective.cpp b/webserv/config/directive/VectorDirective.cpp index 086d410..16f4d27 100644 --- a/webserv/config/directive/VectorDirective.cpp +++ b/webserv/config/directive/VectorDirective.cpp @@ -20,6 +20,10 @@ void VectorDirective::parse(const std::string &value) std::getline(ss, item, ' '); // index indx.html if (!item.empty()) { + if (item.size() > 4096) //TODO: use PATH_MAX or NAME_MAX where appropriate + { + throw std::invalid_argument("VectorDirective: string value exceeds maximum length"); + } value_.push_back(item); } } diff --git a/webserv/http/HttpRequest.cpp b/webserv/http/HttpRequest.cpp index 352f0a8..92262ea 100644 --- a/webserv/http/HttpRequest.cpp +++ b/webserv/http/HttpRequest.cpp @@ -47,6 +47,18 @@ void HttpRequest::setState(State state) state_ = State::ParseError; return; } + if (target_.starts_with("http://") || target_.starts_with("https://")) + { + size_t pos = target_.find('/', 8); // Skip "http://" or "https://" + if (pos == std::string::npos) + { + target_ = "/"; + } + else + { + target_ = target_.substr(pos); + } + } uri_ = std::make_unique(*this, *serverConfig); } state_ = state; @@ -176,6 +188,13 @@ bool HttpRequest::parseBufferforHeaders() state_ = State::Body; return true; } + if (headers_.has("X-Folded-Header")) + { + Log::debug("HttpRequest::parseBuffer() in state Headers with folded headers"); + client_->getHttpResponse().setError(400); + setState(State::ParseError); + return false; + } if (this->headers_.has("Transfer-Encoding") && this->headers_.get("Transfer-Encoding") == "chunked") { Log::debug("HttpRequest::parseBuffer() in state Headers with chunked encoding");