Skip to content

Change handling of invalid JSON and XML request body #177

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
wants to merge 1 commit into
base: waf_nginx
Choose a base branch
from
Open

Change handling of invalid JSON and XML request body #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions apache2/apache2_io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) {
msr_log(msr, 4, "Input filter: Reading request body.");
}
if (modsecurity_request_body_start(msr, error_msg) < 0) {
return -1;
return BODY_PARSER_ERR_GENERIC;
}

finished_reading = 0;
Expand Down Expand Up @@ -226,7 +226,7 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) {
return -2;
default :
*error_msg = apr_psprintf(msr->mp, "Error reading request body: %s", get_apr_error(msr->mp, rc));
return -1;
return BODY_PARSER_ERR_GENERIC;
}
}

Expand Down Expand Up @@ -321,7 +321,7 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) {
}

if((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT))
return -1;
return BODY_PARSER_ERR_GENERIC;
}

}
Expand Down
7 changes: 1 addition & 6 deletions apache2/mod_security2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,19 +1071,14 @@ static int hook_request_late(request_rec *r) {
}
break;
case -6 : /* EOF when reading request body. */
if (my_error_msg != NULL) {
msr_log(msr, 4, "%s", my_error_msg);
}
r->connection->keepalive = AP_CONN_CLOSE;
return HTTP_BAD_REQUEST;
break;
case -7 : /* Partial recieved */
if (my_error_msg != NULL) {
msr_log(msr, 4, "%s", my_error_msg);
}
r->connection->keepalive = AP_CONN_CLOSE;
return HTTP_BAD_REQUEST;
break;
case BODY_PARSER_ERR_INVALID_BODY : /* Error Parsing Body. To be handled by modsec rules */
default :
/* allow through */
break;
Expand Down
5 changes: 5 additions & 0 deletions apache2/modsecurity.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,11 @@ extern DSOLOCAL char *msc_waf_lock_group;

#define NBSP 160


#define BODY_PARSER_ERR_GENERIC -1
#define BODY_PARSER_ERR_INVALID_BODY -8
#define BODY_PARSER_OK_SUCCESS 1

struct rule_exception {
int type;
const char *param;
Expand Down
6 changes: 3 additions & 3 deletions apache2/msc_json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ int json_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char
int json_complete(modsec_rec *msr, char **error_msg) {
char *json_data = (char *) NULL;

if (error_msg == NULL) return -1;
if (error_msg == NULL) return BODY_PARSER_ERR_GENERIC;
*error_msg = NULL;

/* Wrap up the parsing process */
Expand All @@ -301,10 +301,10 @@ int json_complete(modsec_rec *msr, char **error_msg) {
char *yajl_err = yajl_get_error(msr->json->handle, 0, NULL, 0);
*error_msg = apr_pstrdup(msr->mp, yajl_err);
yajl_free_error(msr->json->handle, yajl_err);
return -1;
return BODY_PARSER_ERR_INVALID_BODY;
}

return 1;
return BODY_PARSER_OK_SUCCESS;
}

/**
Expand Down
18 changes: 10 additions & 8 deletions apache2/msc_reqbody.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,45 +697,47 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) {
if (msr->txcfg->debuglog_level >= 4) {
msr_log(msr, 4, "%s", *error_msg);
}
return -1;
return BODY_PARSER_ERR_INVALID_BODY;
}

if (multipart_get_arguments(msr, "BODY", msr->arguments) < 0) {
*error_msg = "Multipart parsing error: Failed to retrieve arguments.";
msr->msc_reqbody_error = 1;
msr->msc_reqbody_error_msg = *error_msg;
msr_log(msr, 2, "%s", *error_msg);
return -1;
return BODY_PARSER_ERR_INVALID_BODY;
}
}
else if (strcmp(msr->msc_reqbody_processor, "JSON") == 0) {
#ifdef WITH_YAJL
if (json_complete(msr, &my_error_msg) < 0 && msr->msc_reqbody_length > 0) {
int json_parser_response = json_complete(msr, &my_error_msg);
if (json_parser_response < 0 && msr->msc_reqbody_length > 0) {
*error_msg = apr_psprintf(msr->mp, "JSON parser error: %s", my_error_msg);
msr->msc_reqbody_error = 1;
msr->msc_reqbody_error_msg = *error_msg;
msr_log(msr, 2, "%s", *error_msg);
return -1;
return json_parser_response;
}
#else
*error_msg = apr_psprintf(msr->mp, "JSON support was not enabled");
msr->msc_reqbody_error = 1;
msr->msc_reqbody_error_msg = *error_msg;
msr_log(msr, 2, "%s", *error_msg);
return -1;
return BODY_PARSER_ERR_GENERIC;
#endif

}
else if (strcmp(msr->msc_reqbody_processor, "URLENCODED") == 0) {
return modsecurity_request_body_end_urlencoded(msr, error_msg);
}
else if (strcmp(msr->msc_reqbody_processor, "XML") == 0) {
if (xml_complete(msr, &my_error_msg) < 0) {
int xml_parser_response = xml_complete(msr, &my_error_msg);
if (xml_parser_response < 0) {
*error_msg = apr_psprintf(msr->mp, "XML parser error: %s", my_error_msg);
msr->msc_reqbody_error = 1;
msr->msc_reqbody_error_msg = *error_msg;
msr_log(msr, 2, "%s", *error_msg);
return -1;
return xml_parser_response;
}
}
} else if (msr->txcfg->reqbody_buffering != REQUEST_BODY_FORCEBUF_OFF) {
Expand All @@ -746,7 +748,7 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) {
/* Note the request body no files length. */
msr_log(msr, 4, "Request body no files length: %" APR_SIZE_T_FMT, msr->msc_reqbody_no_files_length);

return 1;
return BODY_PARSER_OK_SUCCESS;
}

/**
Expand Down
18 changes: 9 additions & 9 deletions apache2/msc_xml.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ int xml_init(modsec_rec *msr, char **error_msg) {
entity = xmlParserInputBufferCreateFilenameDefault(xml_unload_external_entity);
}

return 1;
return BODY_PARSER_OK_SUCCESS;
}

#if 0
Expand All @@ -59,7 +59,7 @@ static void xml_receive_sax_error(void *data, const char *msg, ...) {
* Feed one chunk of data to the XML parser.
*/
int xml_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char **error_msg) {
if (error_msg == NULL) return -1;
if (error_msg == NULL) return BODY_PARSER_ERR_GENERIC;
*error_msg = NULL;

/* We want to initialise our parsing context here, to
Expand Down Expand Up @@ -87,7 +87,7 @@ int xml_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char
msr->xml->parsing_ctx = xmlCreatePushParserCtxt(NULL, NULL, buf, size, "body.xml");
if (msr->xml->parsing_ctx == NULL) {
*error_msg = apr_psprintf(msr->mp, "XML: Failed to create parsing context.");
return -1;
return BODY_PARSER_ERR_GENERIC;
}
} else {

Expand All @@ -96,18 +96,18 @@ int xml_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char
xmlParseChunk(msr->xml->parsing_ctx, buf, size, 0);
if (msr->xml->parsing_ctx->wellFormed != 1) {
*error_msg = apr_psprintf(msr->mp, "XML: Failed parsing document.");
return -1;
return BODY_PARSER_ERR_INVALID_BODY;
}
}

return 1;
return BODY_PARSER_OK_SUCCESS;
}

/**
* Finalise XML parsing.
*/
int xml_complete(modsec_rec *msr, char **error_msg) {
if (error_msg == NULL) return -1;
if (error_msg == NULL) return BODY_PARSER_ERR_GENERIC;
*error_msg = NULL;

/* Only if we have a context, meaning we've done some work. */
Expand All @@ -126,11 +126,11 @@ int xml_complete(modsec_rec *msr, char **error_msg) {

if (msr->xml->well_formed != 1) {
*error_msg = apr_psprintf(msr->mp, "XML: Failed parsing document.");
return -1;
return BODY_PARSER_ERR_INVALID_BODY;
}
}

return 1;
return BODY_PARSER_OK_SUCCESS;
}

/**
Expand All @@ -149,5 +149,5 @@ apr_status_t xml_cleanup(modsec_rec *msr) {
msr->xml->doc = NULL;
}

return 1;
return BODY_PARSER_OK_SUCCESS;
}