Re: Thanks to Debian OpenSSL developers
Bodo Moeller <bmoeller@acm.org> writes:
> This much, by the way, should be very clear to anyone who has read the
> OpenSSL PRNG's source code comments ;-) Anyone who'd look at the
> calling code responsible for the Valgrind warning would have found
> a comment regarding this peculiar behavior. An attempt to understand
> what is going on locally based on just a single line, however, clearly
> is doomed. But even looking just at the single function would have
> shown that the modified version of ssleay_rand_add() doesn't ever
> dereference or pass the "buf" pointer; this should strike as odd even
> if you don't read any of the comments.
And that is exactly why I can't understand how this patch was ever
made nor commented as good on the openssl ML.
It might (and everyone having done security audits knows it is) be
hard to understand the implications of commenting out that line and
grasp the catastrophic effect it has without hindsight. But just
looking at the problem the patch was ment to address and that one
function it is clear that the patch is wrong.
Before you could judge if commenting that line is harmfull or not you
would have to trace every single instance of the function being called
and decide if the buf passed as argument is uninitialized there. Only
when you checked all invocations and prooved to yourself that they all
pass an uninitialized buffer would commenting out that line be an
option. But if you identified all the callers then why not fix them?
You never solve an uninitialized variable problem in a function that
gets that variable as argument. You fix the caller of that function.
> Of course, mistakes can always happen anyway, and to anyone. The
> motto "never fix a bug you don't understand" will only help you out if
> you are aware that you don't understand the bug -- not if you think
> you understand, but actually misunderstand.
I totaly get how you get off by one errors or + and - swapped or
similar. But this mis-fix was already wrong on how it was attempting
to solve the issue in questions. That's why I don't get how at least 2
people messed up like that.
That it also has such a catastrophic effect doesn't even play into
that. To see the effect you have to understand a lot about the code,
intention and algorithms involed. And like most security issues that
is rather complicated and mistake prone. So let me repeated it. I'm
not appaled that nobody saw the effect. I'm shocked that nobody saw
that the way the patch tries to fix uninitialized memory access is
totaly wrong.
> Bodo
MfG
Goswin
Reply to: