Skip to content

Make ModSecurity IIS work with SecStreamInBodyInspection option disabled to prevent memory leak #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 14, 2019
Merged

Make ModSecurity IIS work with SecStreamInBodyInspection option disabled to prevent memory leak #104

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0795b9
Only set APR request body if non-empty.
vkrivopalov Mar 12, 2019
3fa2cac
Explicitly copy request body into Apache structures for any mode.
vkrivopalov Mar 12, 2019
87bd3ad
Do not enforce stream in-body inspection flag for IIS.
vkrivopalov Mar 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 53 additions & 64 deletions iis/mymodule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,58 +448,62 @@ static HRESULT SaveRequestBodyToRequestRec(RequestStoredContext* rsc)
APR_BRIGADE_INSERT_TAIL(brigade, bucket);
}

apr_bucket* e = apr_bucket_eos_create(conn->bucket_alloc);
if (e == nullptr)
{
return E_OUTOFMEMORY;
}
APR_BRIGADE_INSERT_TAIL(brigade, e);
if (!APR_BRIGADE_EMPTY(brigade)) {
apr_bucket* e = apr_bucket_eos_create(conn->bucket_alloc);
if (e == nullptr)
{
return E_OUTOFMEMORY;
}
APR_BRIGADE_INSERT_TAIL(brigade, e);

modsecSetBodyBrigade(aprRequest, brigade);
modsecSetBodyBrigade(aprRequest, brigade);

apr_off_t contentLength = 0;
apr_status_t status = apr_brigade_length(brigade, FALSE, &contentLength);
if (status != APR_SUCCESS)
{
return E_FAIL;
}
apr_off_t contentLength = 0;
apr_status_t status = apr_brigade_length(brigade, FALSE, &contentLength);
if (status != APR_SUCCESS)
{
return E_FAIL;
}

// Remove/Modify Transfer-Encoding header if "chunked" Encoding is set in the request.
// This is to avoid sending both Content-Length and Chunked Transfer-Encoding in the request header.
static const std::string CHUNKED = "chunked";
USHORT encodingLength = 0;
const char* transferEncoding = iisRequest->GetHeader(HttpHeaderTransferEncoding, &encodingLength);
if (transferEncoding)
{
if (CHUNKED.size() == encodingLength &&
std::equal(CHUNKED.cbegin(), CHUNKED.cend(), transferEncoding,
[](char lhs, char rhs) { return std::tolower(lhs) == std::tolower(rhs); }))
// Remove/Modify Transfer-Encoding header if "chunked" Encoding is set in the request.
// This is to avoid sending both Content-Length and Chunked Transfer-Encoding in the request header.
static const std::string CHUNKED = "chunked";
USHORT encodingLength = 0;
const char* transferEncoding = iisRequest->GetHeader(HttpHeaderTransferEncoding, &encodingLength);
if (transferEncoding)
{
iisRequest->DeleteHeader(HttpHeaderTransferEncoding);
if (encodingLength == CHUNKED.size() &&
std::equal(CHUNKED.cbegin(), CHUNKED.cend(), transferEncoding,
[](char lhs, char rhs) { return std::tolower(lhs) == std::tolower(rhs); }))
{
iisRequest->DeleteHeader(HttpHeaderTransferEncoding);
}
}
}

auto contentLengthStr = std::to_string(contentLength);
HRESULT hr = iisRequest->SetHeader(
HttpHeaderContentLength,
contentLengthStr.c_str(),
contentLengthStr.size(),
TRUE);
auto contentLengthStr = std::to_string(contentLength);
HRESULT hr = iisRequest->SetHeader(
HttpHeaderContentLength,
contentLengthStr.c_str(),
contentLengthStr.size(),
TRUE);

if (FAILED(hr))
{
return hr;
}
if (FAILED(hr))
{
return hr;
}

// since we clean the APR pool at the end of OnSendRequest, we must get IIS-managed memory chunk
//
char* requestBuffer = static_cast<char*>(rsc->httpContext->AllocateRequestMemory(contentLength));
status = apr_brigade_flatten(brigade, requestBuffer, reinterpret_cast<apr_size_t*>(&contentLength));
if (status != APR_SUCCESS)
{
return E_FAIL;
// since we clean the APR pool at the end of OnSendRequest, we must get IIS-managed memory chunk
//
char* requestBuffer = static_cast<char*>(rsc->httpContext->AllocateRequestMemory(contentLength));
status = apr_brigade_flatten(brigade, requestBuffer, reinterpret_cast<apr_size_t*>(&contentLength));
if (status != APR_SUCCESS)
{
return E_FAIL;
}
return iisRequest->InsertEntityBody(requestBuffer, contentLength);
}
return iisRequest->InsertEntityBody(requestBuffer, contentLength);

return S_OK;
}


Expand Down Expand Up @@ -953,17 +957,6 @@ CMyHttpModule::OnBeginRequest(IHttpContext* httpContext, IHttpEventProvider* pro

delete apppath;
delete path;

if (config->config->is_enabled == MODSEC_DETECTION_ONLY)
{
modsecSetReadBody(nullptr);
modsecSetWriteBody(nullptr);
}
else
{
modsecSetReadBody(ReadBodyCallback);
modsecSetWriteBody(WriteBodyCallback);
}
}

ConnRecPtr connRec = MakeConnReq();
Expand All @@ -979,11 +972,6 @@ CMyHttpModule::OnBeginRequest(IHttpContext* httpContext, IHttpEventProvider* pro
rsc->responseProcessingEnabled = (config->config->resbody_access == 1);
RequestStoredContext* context = rsc.get();

// on IIS we force input stream inspection flag, because its absence does not add any performance gain
// it's because on IIS request body must be restored each time it was read
//
modsecSetConfigForIISRequestBody(r);

StoreIISContext(r, rsc.get());

httpContext->GetModuleContextContainer()->SetModuleContext(rsc.release(), g_pModuleContext);
Expand Down Expand Up @@ -1214,13 +1202,14 @@ CMyHttpModule::OnBeginRequest(IHttpContext* httpContext, IHttpEventProvider* pro
#endif
c->remote_host = NULL;

hr = SaveRequestBodyToRequestRec(context);
if (FAILED(hr)) {
context->provider->SetErrorStatus(hr);
return RQ_NOTIFICATION_FINISH_REQUEST;
}

if (config->config->is_enabled == MODSEC_DETECTION_ONLY)
{
hr = SaveRequestBodyToRequestRec(context);
if (FAILED(hr)) {
context->provider->SetErrorStatus(hr);
return RQ_NOTIFICATION_FINISH_REQUEST;
}
// We post the processing task to the thread pool to happen in the background.
// We store the future to track and wait for this processing in case if we also
// need to process the response because we need request processing to finish
Expand Down