lib/sbi: Fix unsafe memory handling in access_handler()

The HTTP upload handling in access_handler() used ogs_malloc() and
ogs_realloc() results directly assigned to request->http.content and
checked with ogs_assert(). On allocation failure this could abort the
process, leading to a potential denial-of-service condition. The pattern
was similar to the issue previously fixed in on_data_chunk_recv()
(CVE-2022-3299).

This change introduces a temporary pointer for memory allocation and
updates request->http.content only after successful allocation. It also
adds overflow-safe length checks before resizing the buffer and removes
assert-based error handling in favor of graceful failure.

This prevents process termination on allocation failure and aligns the
memory handling logic with the hardened implementation used in
nghttp2-based handlers.

Issues: #4387
This commit is contained in:
Sukchan Lee 2026-04-06 17:42:59 +09:00
parent a51df637ac
commit 5ab76f2bea

View file

@ -543,8 +543,7 @@ static _MHD_Result access_handler(
if (ogs_sbi_header_get(request->http.headers, "Content-Length") ||
ogs_sbi_header_get(request->http.headers, "Transfer-Encoding")) {
// FIXME : check if POST_DATA is on MHD_POSTDATA_KIND
/* FIXME : check if POST_DATA is on MHD_POSTDATA_KIND */
return MHD_YES;
}
@ -552,29 +551,43 @@ static _MHD_Result access_handler(
}
if (*upload_data_size != 0) {
char *content = NULL;
size_t offset = 0;
size_t new_length = 0;
if (*upload_data_size > OGS_MAX_SDU_LEN -
request->http.content_length) {
ogs_error("Payload too large : Content-Length[%d], "
"upload_data_size[%d]",
(int)request->http.content_length,
(int)*upload_data_size);
*upload_data_size = 0;
return MHD_NO;
}
new_length = request->http.content_length + *upload_data_size;
offset = request->http.content_length;
if (request->http.content == NULL) {
request->http.content_length = *upload_data_size;
request->http.content =
(char*)ogs_malloc(request->http.content_length + 1);
ogs_assert(request->http.content);
ogs_assert(request->http.content_length == 0);
content = ogs_malloc(new_length + 1);
} else {
offset = request->http.content_length;
if ((request->http.content_length +
*upload_data_size) > OGS_MAX_SDU_LEN) {
ogs_error("Overflow : Content-Length[%d], upload_data_size[%d]",
(int)request->http.content_length,
(int)*upload_data_size);
*upload_data_size = 0;
return MHD_YES;
}
request->http.content_length += *upload_data_size;
request->http.content = (char *)ogs_realloc(
request->http.content, request->http.content_length + 1);
ogs_assert(request->http.content);
content = ogs_realloc(request->http.content, new_length + 1);
}
if (!content) {
ogs_error("Memory allocation failed : Content-Length[%d], "
"upload_data_size[%d]",
(int)request->http.content_length,
(int)*upload_data_size);
*upload_data_size = 0;
return MHD_NO;
}
request->http.content = content;
request->http.content_length = new_length;
memcpy(request->http.content + offset, upload_data, *upload_data_size);
request->http.content[request->http.content_length] = '\0';
*upload_data_size = 0;