Skip to content

Commit 0e4728f

Browse files
hnakamurhnakamur
committed
Accept partial epilogue in body larger than limit for ProcessPartial
But reject incomplete epilogue when body fits in limit.
1 parent e9dca3c commit 0e4728f

2 files changed

Lines changed: 308 additions & 1 deletion

File tree

‎apache2/msc_multipart.c‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1023,13 +1023,44 @@ int multipart_complete(modsec_rec *msr, char **error_msg) {
10231023
* processed yet) in the buffer.
10241024
*/
10251025
if (msr->mpd->buf_contains_line) {
1026-
if ( ((unsigned int)(MULTIPART_BUF_SIZE - msr->mpd->bufleft) == (4 + strlen(msr->mpd->boundary)))
1026+
/*
1027+
* Note that the buffer may end with the final boundary followed by only CR,
1028+
* coming from the [CRLF epilogue], when allow_process_partial == 1 (which is
1029+
* set when SecRequestBodyLimitAction is ProcessPartial and the request body
1030+
* length exceeds SecRequestBodyLimit).
1031+
*
1032+
* The following definitions are copied from RFC 2046:
1033+
*
1034+
* dash-boundary := "--" boundary
1035+
*
1036+
* delimiter := CRLF dash-boundary
1037+
*
1038+
* close-delimiter := delimiter "--"
1039+
*
1040+
* multipart-body := [preamble CRLF]
1041+
* dash-boundary transport-padding CRLF
1042+
* body-part *encapsulation
1043+
* close-delimiter transport-padding
1044+
* [CRLF epilogue]
1045+
*/
1046+
unsigned int buf_data_len = (unsigned int)(MULTIPART_BUF_SIZE - msr->mpd->bufleft);
1047+
size_t final_boundary_len = 4 + strlen(msr->mpd->boundary);
1048+
if ( (buf_data_len >= final_boundary_len)
10271049
&& (*(msr->mpd->buf) == '-')
10281050
&& (*(msr->mpd->buf + 1) == '-')
10291051
&& (strncmp(msr->mpd->buf + 2, msr->mpd->boundary, strlen(msr->mpd->boundary)) == 0)
10301052
&& (*(msr->mpd->buf + 2 + strlen(msr->mpd->boundary)) == '-')
10311053
&& (*(msr->mpd->buf + 2 + strlen(msr->mpd->boundary) + 1) == '-') )
10321054
{
1055+
/* If body fits in limit and ends with final boundary plus just CR, reject it. */
1056+
if ( (msr->mpd->allow_process_partial == 0)
1057+
&& (buf_data_len == final_boundary_len + 1)
1058+
&& (*(msr->mpd->buf + final_boundary_len) == '\r') )
1059+
{
1060+
*error_msg = apr_psprintf(msr->mp, "Multipart: Invalid epilogue after final boundary.");
1061+
return -1;
1062+
}
1063+
10331064
if ((msr->mpd->crlf_state_buf_end == 2) && (msr->mpd->flag_lf_line != 1)) {
10341065
msr->mpd->flag_lf_line = 1;
10351066
if (msr->mpd->flag_crlf_line) {

‎tests/regression/config/10-request-directives.t‎

Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,283 @@
643643
# ),
644644
#},
645645

646+
# SecRequestBodyLimitAction ProcessPartial
647+
{
648+
type => "config",
649+
comment => "SecRequestBodyLimitAction ProcessPartial (multipart/just limit - bad_name)",
650+
conf => qq(
651+
SecRuleEngine On
652+
SecDebugLog $ENV{DEBUG_LOG}
653+
SecDebugLogLevel 9
654+
SecRequestBodyAccess On
655+
SecRequestBodyLimitAction ProcessPartial
656+
SecRequestBodyLimit 296
657+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"
658+
SecRule MULTIPART_NAME "bad_name" "id:'200002',phase:2,t:none,deny
659+
),
660+
match_log => {
661+
-error => [ qr/Multipart parsing error: Multipart: Final boundary missing./, 1],
662+
},
663+
match_response => {
664+
status => qr/^403$/,
665+
},
666+
request => new HTTP::Request(
667+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
668+
[
669+
"Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646",
670+
],
671+
normalize_raw_request_data(
672+
q(
673+
-----------------------------69343412719991675451336310646
674+
Content-Disposition: form-data; name="name1"
675+
676+
value1
677+
-----------------------------69343412719991675451336310646
678+
Content-Disposition: form-data; name="bad_name2"
646679
680+
value2
681+
-----------------------------69343412719991675451336310646--),
682+
),
683+
),
684+
},
685+
{
686+
type => "config",
687+
comment => "SecRequestBodyLimitAction ProcessPartial (multipart/greater - bad_name)",
688+
conf => qq(
689+
SecRuleEngine On
690+
SecDebugLog $ENV{DEBUG_LOG}
691+
SecDebugLogLevel 9
692+
SecRequestBodyAccess On
693+
SecRequestBodyLimitAction ProcessPartial
694+
SecRequestBodyLimit 295
695+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"
696+
SecRule MULTIPART_NAME "bad_name" "id:'200002',phase:2,t:none,deny
697+
),
698+
match_log => {
699+
-error => [ qr/Multipart parsing error: Multipart: Final boundary missing./, 1],
700+
},
701+
match_response => {
702+
status => qr/^403$/,
703+
},
704+
request => new HTTP::Request(
705+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
706+
[
707+
"Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646",
708+
],
709+
normalize_raw_request_data(
710+
q(
711+
-----------------------------69343412719991675451336310646
712+
Content-Disposition: form-data; name="name1"
713+
714+
value1
715+
-----------------------------69343412719991675451336310646
716+
Content-Disposition: form-data; name="bad_name2"
717+
718+
value2
719+
-----------------------------69343412719991675451336310646--),
720+
),
721+
),
722+
},
723+
{
724+
type => "config",
725+
comment => "SecRequestBodyLimitAction ProcessPartial (multipart/no epilogue)",
726+
conf => qq(
727+
SecRuleEngine On
728+
SecDebugLog $ENV{DEBUG_LOG}
729+
SecDebugLogLevel 9
730+
SecRequestBodyAccess On
731+
SecRequestBodyLimitAction ProcessPartial
732+
SecRequestBodyLimit 176
733+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"
734+
),
735+
match_log => {
736+
-error => [ qr/Multipart parsing error: Multipart: Final boundary missing./, 1],
737+
},
738+
match_response => {
739+
status => qr/^200$/,
740+
},
741+
request => new HTTP::Request(
742+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
743+
[
744+
"Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646",
745+
],
746+
normalize_raw_request_data(
747+
q(
748+
-----------------------------69343412719991675451336310646
749+
Content-Disposition: form-data; name="name1"
750+
751+
value1
752+
-----------------------------69343412719991675451336310646--),
753+
),
754+
),
755+
},
756+
{
757+
type => "config",
758+
comment => "SecRequestBodyLimitAction ProcessPartial (multipart/CR after limit)",
759+
conf => qq(
760+
SecRuleEngine On
761+
SecDebugLog $ENV{DEBUG_LOG}
762+
SecDebugLogLevel 9
763+
SecRequestBodyAccess On
764+
SecRequestBodyLimitAction ProcessPartial
765+
SecRequestBodyLimit 176
766+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"
767+
),
768+
match_log => {
769+
-error => [ qr/Multipart parsing error: Multipart: Final boundary missing./, 1],
770+
},
771+
match_response => {
772+
status => qr/^200$/,
773+
},
774+
request => new HTTP::Request(
775+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
776+
[
777+
"Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646",
778+
],
779+
normalize_raw_request_data(
780+
q(
781+
-----------------------------69343412719991675451336310646
782+
Content-Disposition: form-data; name="name1"
783+
784+
value1
785+
-----------------------------69343412719991675451336310646--) . "\r",
786+
),
787+
),
788+
},
789+
{
790+
type => "config",
791+
comment => "SecRequestBodyLimitAction ProcessPartial (multipart/CR just in limit)",
792+
conf => qq(
793+
SecRuleEngine On
794+
SecDebugLog $ENV{DEBUG_LOG}
795+
SecDebugLogLevel 9
796+
SecRequestBodyAccess On
797+
SecRequestBodyLimitAction ProcessPartial
798+
SecRequestBodyLimit 177
799+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"
800+
),
801+
match_log => {
802+
-error => [ qr/"Multipart: Invalid epilogue after final boundary."/, 1],
803+
},
804+
match_response => {
805+
status => qr/^400$/,
806+
},
807+
request => new HTTP::Request(
808+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
809+
[
810+
"Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646",
811+
],
812+
normalize_raw_request_data(
813+
q(
814+
-----------------------------69343412719991675451336310646
815+
Content-Disposition: form-data; name="name1"
816+
817+
value1
818+
-----------------------------69343412719991675451336310646--) . "\r",
819+
),
820+
),
821+
},
822+
{
823+
type => "config",
824+
comment => "SecRequestBodyLimitAction ProcessPartial (multipart/CRLF across limit)",
825+
conf => qq(
826+
SecRuleEngine On
827+
SecDebugLog $ENV{DEBUG_LOG}
828+
SecDebugLogLevel 9
829+
SecRequestBodyAccess On
830+
SecRequestBodyLimitAction ProcessPartial
831+
SecRequestBodyLimit 177
832+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"
833+
),
834+
match_log => {
835+
-error => [ qr/Multipart parsing error: Multipart: Final boundary missing./, 1],
836+
},
837+
match_response => {
838+
status => qr/^200$/,
839+
},
840+
request => new HTTP::Request(
841+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
842+
[
843+
"Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646",
844+
],
845+
normalize_raw_request_data(
846+
q(
847+
-----------------------------69343412719991675451336310646
848+
Content-Disposition: form-data; name="name1"
849+
850+
value1
851+
-----------------------------69343412719991675451336310646--
852+
),
853+
),
854+
),
855+
},
856+
{
857+
type => "config",
858+
comment => "SecRequestBodyLimitAction ProcessPartial (multipart/CR before limit, non-LF after)",
859+
conf => qq(
860+
SecRuleEngine On
861+
SecDebugLog $ENV{DEBUG_LOG}
862+
SecDebugLogLevel 9
863+
SecRequestBodyAccess On
864+
SecRequestBodyLimitAction ProcessPartial
865+
SecRequestBodyLimit 177
866+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"
867+
),
868+
match_log => {
869+
-error => [ qr/Multipart parsing error: Multipart: Final boundary missing./, 1],
870+
},
871+
match_response => {
872+
status => qr/^200$/,
873+
},
874+
request => new HTTP::Request(
875+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
876+
[
877+
"Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646",
878+
],
879+
normalize_raw_request_data(
880+
q(
881+
-----------------------------69343412719991675451336310646
882+
Content-Disposition: form-data; name="name1"
883+
884+
value1
885+
-----------------------------69343412719991675451336310646--) . "\rbad epilogue after just CR",
886+
),
887+
),
888+
},
889+
{
890+
type => "config",
891+
comment => "SecRequestBodyLimitAction ProcessPartial (multipart/empty epilogue just in limit)",
892+
conf => qq(
893+
SecRuleEngine On
894+
SecDebugLog $ENV{DEBUG_LOG}
895+
SecDebugLogLevel 9
896+
SecRequestBodyAccess On
897+
SecRequestBodyLimitAction ProcessPartial
898+
SecRequestBodyLimit 178
899+
SecRule REQBODY_ERROR "!\@eq 0" "id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"
900+
),
901+
match_log => {
902+
-error => [ qr/Multipart parsing error: Multipart: Final boundary missing./, 1],
903+
},
904+
match_response => {
905+
status => qr/^200$/,
906+
},
907+
request => new HTTP::Request(
908+
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt",
909+
[
910+
"Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646",
911+
],
912+
normalize_raw_request_data(
913+
q(
914+
-----------------------------69343412719991675451336310646
915+
Content-Disposition: form-data; name="name1"
916+
917+
value1
918+
-----------------------------69343412719991675451336310646--
919+
),
920+
),
921+
),
922+
},
647923
648924
649925

0 commit comments

Comments
 (0)