Skip to content

V3/multipart part header errors #3650

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

Open
airween wants to merge 6 commits into
base: v3/master
Choose a base branch
from
Open

V3/multipart part header errors #3650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e9f6663
fix: fix parser's wrong behavior, align tests
airween Oct 2, 2026
8082be1
fix: change REQBODY_PROCESSOR_ERROR to REQBODY_ERROR
airween Oct 2, 2026
8636f84
chore: apply Copilot and Coderabbit suggestions
airween Oct 2, 2026
0c81b85
fix: set IP if filename field contains space
airween Oct 2, 2026
8b09da8
chore: remove macos-14 from ci.yaml
airween Oct 2, 2026
36ddb18
Merge branch 'v3/master' into v3/multipart-part-header-errors
airween Oct 10, 2026
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
2 changes: 1 addition & 1 deletion modsecurity.conf-recommended
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ SecRule REQBODY_ERROR "!@eq 0" \
SecRule MULTIPART_STRICT_ERROR "!@eq 0" \
"id:'200003',phase:2,t:none,log,deny,status:400, \
msg:'Multipart request body failed strict validation: \
PE %{REQBODY_PROCESSOR_ERROR}, \
RE %{REQBODY_ERROR}, \
BQ %{MULTIPART_BOUNDARY_QUOTED}, \
BW %{MULTIPART_BOUNDARY_WHITESPACE}, \
DB %{MULTIPART_DATA_BEFORE}, \
Expand Down
41 changes: 27 additions & 14 deletions src/request_body_processor/multipart.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
p++;
}
if ((*p != '\'') || (p == start_of_charset)) {
m_flag_invalid_quoting = 1;
return -16; // Must be at least one legit char before ' for start of language
}
m_mpp->m_filename_charset.assign(start_of_charset, (p - start_of_charset));
Expand Down Expand Up @@ -766,7 +767,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
"Multipart: Nul byte in part headers.");

error->assign("Multipart: Nul byte in part headers.");
return false;
return -1;
}
}

Expand Down Expand Up @@ -802,9 +803,10 @@ int Multipart::process_part_header(std::string *error, int offset) {
ms_dbg_a(m_transaction, 1,
"Multipart: Part missing Content-Disposition header.");

m_flag_invalid_part = 1;
error->assign("Multipart: Part missing " \
"Content-Disposition header.");
return false;
return -1;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
header_value = m_mpp->m_headers.at("Content-Disposition").second;

Expand All @@ -821,20 +823,22 @@ int Multipart::process_part_header(std::string *error, int offset) {
"Multipart: Invalid Content-Disposition header ("
+ std::to_string(rc) + "): " + header_value);

m_flag_invalid_part = 1;
error->assign("Multipart: Invalid Content-Disposition header ("
+ std::to_string(rc) + "): " + header_value);
return false;
return -1;
}

if (m_mpp->m_name.empty()) {
ms_dbg_a(m_transaction, 1,
"Multipart: Content-Disposition header missing " \
"name field.");

m_flag_invalid_part = 1;
Comment thread
airween marked this conversation as resolved.
error->assign("Multipart: Content-Disposition header missing " \
"name field.");

return false;
return -1;
}

if (!m_mpp->m_filename.empty() || !m_mpp->m_filenameStar.empty()) {
Expand All @@ -845,13 +849,14 @@ int Multipart::process_part_header(std::string *error, int offset) {
* also we need to check if any of the filename or filename* is present
*/
if (strstr(header_value.c_str(), "filename=") == NULL && strstr(header_value.c_str(), "filename*=") == NULL) {
m_flag_invalid_part = 1;
ms_dbg_a(m_transaction, 1,
"Multipart: Invalid Content-Disposition " \
"header (filename).");

error->assign("Multipart: Invalid Content-Disposition " \
"header (filename).");
return false;
return -1;
Comment thread
airween marked this conversation as resolved.
}

m_mpp->m_type = MULTIPART_FILE;
Expand Down Expand Up @@ -886,7 +891,7 @@ int Multipart::process_part_header(std::string *error, int offset) {

error->assign("Multipart: Invalid part header " \
"(folding error).");
return false;
return -1;
}

/* locate the beginning of data */
Expand Down Expand Up @@ -920,7 +925,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
ms_dbg_a(m_transaction, 1, "Multipart: Part header too long.");

error->assign("Multipart: Part header too long.");
return false;
return -1;
}

m_mpp->m_last_header_line = m_mpp->m_last_header_name + ": " + new_value;
Expand Down Expand Up @@ -949,7 +954,7 @@ int Multipart::process_part_header(std::string *error, int offset) {

error->assign("Multipart: Invalid part header " \
"(colon missing): " + std::string(m_buf));
return false;
return -1;
}

/* extract header name */
Expand All @@ -961,7 +966,7 @@ int Multipart::process_part_header(std::string *error, int offset) {

error->assign("Multipart: Invalid part header " \
"(header name missing).");
return false;
return -1;
}

/* check if multipart header contains any invalid characters */
Expand All @@ -971,7 +976,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
ms_dbg_a(m_transaction, 1,
"Multipart: Invalid part header (contains invalid character).");
error->assign("Multipart: Invalid part header (contains invalid character).");
return false;
return -1;
}

/* extract the value value */
Expand All @@ -990,7 +995,9 @@ int Multipart::process_part_header(std::string *error, int offset) {
ms_dbg_a(m_transaction, 1,
"Multipart: Duplicate part header: " \
+ header_name + ".");
return false;
error->assign("Multipart: Duplicate part header: " \
+ header_name + ".");
return -1;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

m_mpp->m_last_header_name.assign(header_name);
Expand All @@ -1011,7 +1018,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
}
}

return true;
return 1;
}


Expand Down Expand Up @@ -1256,13 +1263,19 @@ int Multipart::multipart_complete(std::string *error) {
if (m_is_complete == 0) {
ms_dbg_a(m_transaction, 1,
"Multipart: Final boundary missing.");
error->assign("Multipart: Final boundary missing.");
// keep the error of the processing step, if there was any
if (error->empty()) {
error->assign("Multipart: Final boundary missing.");
}
return false;
}
} else {
ms_dbg_a(m_transaction, 1,
"Multipart: No boundaries found in payload.");
error->assign("Multipart: No boundaries found in payload.");
// keep the error of the processing step, if there was any
if (error->empty()) {
error->assign("Multipart: No boundaries found in payload.");
}
return false;
}
}
Expand Down
Loading
Loading