orbisai0security commented on code in PR #73:
URL: https://github.com/apache/apr/pull/73#discussion_r3256694007
##########
buffer/apr_buffer.c:
##########
@@ -245,6 +245,9 @@ APR_DECLARE(apr_status_t) apr_buffer_arraydup(apr_buffer_t
**out,
for (i = 0; i < nelts; i++) {
/* absolute value is size of mem buffer including optional terminating
zero */
+ if (src->size > APR_BUFFER_MAX) {
+ return APR_EINVAL;
+ }
apr_size_t size = src->size + src->zero_terminated;
Review Comment:
You're right — this check is actually unreachable. The `size` field is a
63-bit bitfield (on 64-bit), so its maximum representable value is exactly
`APR_BUFFER_MAX` (`APR_SIZE_MAX/2`). The condition `src->size > APR_BUFFER_MAX`
can never be true.
I've removed both APR_BUFFER_MAX checks from the PR. The reworked patch
focuses solely on the NULL alloc check, which addresses real undefined
behaviour.
##########
buffer/apr_buffer.c:
##########
@@ -294,9 +297,15 @@ APR_DECLARE(apr_buffer_t *) apr_buffer_cpy(apr_buffer_t
*dst,
else {
/* absolute value is size of mem buffer including optional terminating
zero */
+ if (src->size > APR_BUFFER_MAX) {
+ return NULL;
+ }
apr_size_t size = src->size + src->zero_terminated;
void *mem = alloc(ctx, size);
+ if (!mem) {
+ return NULL;
+ }
memcpy(mem, src->d.mem, size);
Review Comment:
I've updated the `apr_buffer_cpy()` documentation in the header to
explicitly state that NULL is returned when `alloc` returns NULL. This makes
the API contract clear and consistent with `apr_buffer_arraydup()` which
documents APR_ENOMEM on allocation failure.
A test exercising this path is also included.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
