Re: [PR] apr_buffer_cpy: return NULL on allocation fai lure to avoid undefined behaviour [apr]

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]