On 22. 5. 26 10:35, Daniel Sahlberg wrote:
Den sön 17 maj 2026 kl 19:47 skrev Daniel Sahlberg
<[email protected]>:
Den sön 17 maj 2026 kl 19:14 skrev orbisai0security (via GitHub)
<[email protected]>:
>
>
> orbisai0security commented on PR #73:
> URL: https://github.com/apache/apr/pull/73#issuecomment-4471676980
>
> Thanks for the review. I agree that the current description
overstates the issue and incorrectly frames this as a confirmed
critical overflow.
>
> I’ll revise the PR to narrow it to defensive hardening only.
In particular, I’ll remove the “five memcpy calls” / “critical
severity” language and keep only the allocation-failure guard
before memcpy(), since calling memcpy with a NULL destination
after alloc() failure would be undefined behaviour.
>
> For the APR_BUFFER_MAX checks, I understand your point that
they do not prove that src->d.mem is actually backed by src->size
bytes, so they do not fix the claimed issue. I’m happy to drop
those from this PR unless you think they are still useful as a
separate invariant check.
>
> Would a smaller patch focused only on the alloc() NULL check,
with tests/docs adjusted for expected behaviour, be acceptable?
I will refer this question to the rest of dev@
Cheers,
Daniel
Any takers? This is way above my paygrade :-)
In the meantime I digged a bit further and I see that apr_pmemdup
seems to do the same, it allocates a buffer and then immediately uses it:
...
res = apr_palloc(a, n);
memcpy(res, m, n);
...
I realise that you can have an abort_fn in the pool and if you use
apr_palloc as the alloc function you will get a callback, but you
can't really avoid the memcpy(NULL, ...).
The abort callback's only purpose is to allow the application to perform
a clean-ish/safe-ish shutdown.
But the fact that the pattern in apr_pmemdup is the same as in
apr_buffer_cpy make me believe this is intentional. Or we have two
bugs ;-)
We use the same pattern throughout. APR does not attempt to recover from
OOM conditions, such recovery is possible only in very limited
circumstances and those depend on the application. On modern platforms
that allow memory overcommit, the allocation may "fail" long after
apr_palloc() returns a valid pointer and there's nothing the program can
do about it. The abort callback is really only used in the rare edge
cases where the system actually tells us that the allocation (will)
fail(ed).
All that said ... the pool functions do return NULL when the abort
callback isn't set. So I guess we should be checking for NULL in our
code and ... then what? Abort? There are platforms when NULL dereference
doesn't cause an immediate crash, so this is tricky.
-- Brane
