Skip to content

Commit 7e0f58f

Browse files
committed
Do partial processing if action is ProcessPartial and request body size exceeds limit
In other words, if the request body size is equal or less than the RequestBodySizeLimit, the request processor requires a well-formed XML and does not allow a partial JSON, even if SecRequestBodyLimitAction is set to ProcessPartial.
1 parent b77a1a7 commit 7e0f58f

File tree

8 files changed

+345
-32
lines changed

8 files changed

+345
-32
lines changed

headers/modsecurity/transaction.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,11 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
645645
* the web server (connector) log.
646646
*/
647647
void *m_logCbData;
648+
649+
/**
650+
* Whether the request body was bigger than RequestBodyLimit.
651+
*/
652+
bool m_requestBodyLimitExceeded;
648653
};
649654

650655

src/request_body_processor/json.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ namespace RequestBodyProcessor {
2929
static const double json_depth_limit_default = 10000.0;
3030
static const char* json_depth_limit_exceeded_msg = ". Parsing depth limit exceeded";
3131

32-
JSON::JSON(Transaction *transaction, unsigned int allow_partial_values)
32+
JSON::JSON(Transaction *transaction)
3333
: m_transaction(transaction),
3434
m_handle(NULL),
3535
m_current_key(""),
@@ -69,8 +69,6 @@ JSON::JSON(Transaction *transaction, unsigned int allow_partial_values)
6969
* TODO: make UTF8 validation optional, as it depends on Content-Encoding
7070
*/
7171
m_handle = yajl_alloc(&callbacks, NULL, this);
72-
73-
yajl_config(m_handle, yajl_allow_partial_values, allow_partial_values);
7472
}
7573

7674

@@ -84,7 +82,8 @@ JSON::~JSON() {
8482
}
8583

8684

87-
bool JSON::init() {
85+
bool JSON::init(unsigned int allow_partial_values) {
86+
yajl_config(m_handle, yajl_allow_partial_values, allow_partial_values);
8887
return true;
8988
}
9089

src/request_body_processor/json.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,10 @@ class JSONContainerMap : public JSONContainer {
5757

5858
class JSON {
5959
public:
60-
explicit JSON(Transaction *transaction, unsigned int allow_partial_values = 0);
60+
explicit JSON(Transaction *transaction);
6161
~JSON();
6262

63-
static bool init();
63+
bool init(unsigned int allow_partial_values = 0);
6464
bool processChunk(const char *buf, unsigned int size, std::string *err);
6565
bool complete(std::string *err);
6666

src/request_body_processor/xml.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ extern "C" {
149149
}
150150
}
151151

152-
XML::XML(Transaction *transaction, bool require_well_formed)
153-
: m_transaction(transaction), m_require_well_formed(require_well_formed) {
152+
XML::XML(Transaction *transaction)
153+
: m_transaction(transaction), m_require_well_formed(false) {
154154
m_data.doc = NULL;
155155
m_data.parsing_ctx = NULL;
156156
m_data.sax_handler = NULL;
@@ -171,7 +171,8 @@ XML::~XML() {
171171
}
172172
}
173173

174-
bool XML::init() {
174+
bool XML::init(bool require_well_formed) {
175+
m_require_well_formed = require_well_formed;
175176
//xmlParserInputBufferCreateFilenameFunc entity;
176177
if (m_transaction->m_rules->m_secXMLExternalEntity
177178
== RulesSetProperties::TrueConfigBoolean) {

src/request_body_processor/xml.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ typedef struct xml_data xml_data;
8585

8686
class XML {
8787
public:
88-
explicit XML(Transaction *transaction, bool require_well_formed = true);
88+
explicit XML(Transaction *transaction);
8989
~XML();
90-
bool init();
90+
bool init(bool require_well_formed = true);
9191
bool processChunk(const char *buf, unsigned int size, std::string *err);
9292
bool complete(std::string *err);
9393
static xmlParserInputBufferPtr unloadExternalEntity(const char *URI,

src/transaction.cc

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -138,20 +138,19 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, const char *id,
138138
ms->m_session_collection, ms->m_user_collection,
139139
ms->m_resource_collection),
140140
#ifdef WITH_LIBXML2
141-
m_xml(std::make_unique<RequestBodyProcessor::XML>(this,
142-
this->m_rules->m_requestBodyLimitAction != RulesSet::BodyLimitAction::ProcessPartialBodyLimitAction)),
141+
m_xml(std::make_unique<RequestBodyProcessor::XML>(this)),
143142
#else
144143
m_xml(nullptr),
145144
#endif
146145
#ifdef WITH_YAJL
147-
m_json(std::make_unique<RequestBodyProcessor::JSON>(this,
148-
this->m_rules->m_requestBodyLimitAction == RulesSet::BodyLimitAction::ProcessPartialBodyLimitAction)),
146+
m_json(std::make_unique<RequestBodyProcessor::JSON>(this)),
149147
#else
150148
m_json(nullptr),
151149
#endif
152150
m_secRuleEngine(RulesSetProperties::PropertyNotSetRuleEngine),
153151
m_secXMLParseXmlIntoArgs(rules->m_secXMLParseXmlIntoArgs),
154152
m_logCbData(logCbData),
153+
m_requestBodyLimitExceeded(false),
155154
TransactionAnchoredVariables(this) {
156155
m_variableUrlEncodedError.set("0", 0);
157156
m_variableMscPcreError.set("0", 0);
@@ -689,27 +688,33 @@ int Transaction::processRequestBody() {
689688
std::unique_ptr<std::string> a = m_variableRequestHeaders.resolveFirst(
690689
"Content-Type");
691690

691+
bool is_process_partial = (m_rules->m_requestBodyLimitAction
692+
== RulesSet::BodyLimitAction::ProcessPartialBodyLimitAction);
693+
692694
bool requestBodyNoFilesLimitExceeded = false;
693695
if ((m_requestBodyType == WWWFormUrlEncoded) ||
694696
(m_requestBodyProcessor == JSONRequestBody) ||
695697
(m_requestBodyProcessor == XMLRequestBody)) {
696698
if ((m_rules->m_requestBodyNoFilesLimit.m_set)
697699
&& (m_requestBody.str().size() > m_rules->m_requestBodyNoFilesLimit.m_value)) {
698-
m_variableReqbodyError.set("1", 0);
699-
m_variableReqbodyErrorMsg.set("Request body excluding files is bigger than the maximum expected.", 0);
700-
m_variableInboundDataError.set("1", m_variableOffset);
701-
ms_dbg(5, "Request body excluding files is bigger than the maximum expected. Limit: " \
702-
+ std::to_string(m_rules->m_requestBodyNoFilesLimit.m_value));
700+
if (!is_process_partial) {
701+
m_variableReqbodyError.set("1", 0);
702+
m_variableReqbodyErrorMsg.set("Request body excluding files is bigger than the maximum expected.", 0);
703+
m_variableInboundDataError.set("1", m_variableOffset);
704+
ms_dbg(5, "Request body excluding files is bigger than the maximum expected. Limit: " \
705+
+ std::to_string(m_rules->m_requestBodyNoFilesLimit.m_value));
706+
}
703707
requestBodyNoFilesLimitExceeded = true;
704-
}
708+
}
705709
}
706710

707711
#ifdef WITH_LIBXML2
708712
if (m_requestBodyProcessor == XMLRequestBody) {
709713
// large size might cause issues in the parsing itself; omit if exceeded
710-
if (!requestBodyNoFilesLimitExceeded) {
714+
if (!requestBodyNoFilesLimitExceeded || is_process_partial) {
711715
std::string error;
712-
if (m_xml->init() == true) {
716+
bool require_well_formed = !(is_process_partial && m_requestBodyLimitExceeded);
717+
if (m_xml->init(require_well_formed) == true) {
713718
m_xml->processChunk(m_requestBody.str().c_str(),
714719
m_requestBody.str().size(),
715720
&error);
@@ -735,12 +740,13 @@ int Transaction::processRequestBody() {
735740
if (m_requestBodyProcessor == JSONRequestBody) {
736741
#endif
737742
// large size might cause issues in the parsing itself; omit if exceeded
738-
if (!requestBodyNoFilesLimitExceeded) {
743+
if (!requestBodyNoFilesLimitExceeded || is_process_partial) {
739744
std::string error;
740745
if (m_rules->m_requestBodyJsonDepthLimit.m_set) {
741746
m_json->setMaxDepth(m_rules->m_requestBodyJsonDepthLimit.m_value);
742747
}
743-
if (m_json->init() == true) {
748+
unsigned int allow_partial_values = is_process_partial && m_requestBodyLimitExceeded;
749+
if (m_json->init(allow_partial_values) == true) {
744750
m_json->processChunk(m_requestBody.str().c_str(),
745751
m_requestBody.str().size(),
746752
&error);
@@ -930,6 +936,7 @@ int Transaction::appendRequestBody(const unsigned char *buf, size_t len) {
930936

931937
if (this->m_rules->m_requestBodyLimit.m_value > 0
932938
&& this->m_rules->m_requestBodyLimit.m_value < len + current_size) {
939+
m_requestBodyLimitExceeded = true;
933940
m_variableInboundDataError.set("1", m_variableOffset);
934941
ms_dbg(5, "Request body is bigger than the maximum expected.");
935942

test/regression/regression.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,7 @@ void perform_unit_test(const ModSecurityTest<RegressionTest> &test,
311311
modsec_transaction.appendResponseBody(
312312
(unsigned char *)t->response_body.c_str(),
313313
t->response_body.size());
314+
314315
modsec_transaction.processResponseBody();
315316
actions(&r, &modsec_transaction, &context.m_server_log);
316317

0 commit comments

Comments
 (0)