The cipher BIO has a nasty bug that can cause some of the final eight
bytes to be lost during decryption. It may affect other BIO's. I'm
using openssl 0.9.4.
The problem is in bio_enc.c, the function enc_read. Consider the
situation in which we enter the function with our buffer ctx->buf empty
(ctx->buf_len == 0). This is perfectly possible if the last call to
enc_read was for exactly the size of our buffer.
We will skip past reading out of the buffer, and arrive at the "read in
some more" loop ("while (outl > 0)") with no bytes in the output buffer
(ret == 0).
Suppose then that the next BIO is at EOF. BIO_read and BIO_should_retry
will both return 0. That puts us in the block starting "ctx->cont=i;",
line 180 in my version. We get the last bytes from CipherFinal into our
buffer, then hit the break. This throws us all the way to the end of
the function, where we return 0, because ret == 0. Uh oh--we just
returned EOF, and we have the bytes from CipherFinal sitting in our
buffer!
The reason that this problem does not occur frequently is that, in most
cases, there are already some bytes in the output bufferwhen we call
CipherFinal. Those bytes will be returned, and when enc_read is called
again, the CipherFinal bytes will be read from the buffer, and all will
end happily.
It's possible that this problem afflicts the base64 BIO as well, but I'm
uncertain because it doesn't even call DecodeFinal, so clearly it knows
something about the innards of the base64 EVP code that I don't.
I think that in bio_enc.c a solution is simply not to break after
calling CipherFinal. The situation at that point is really no different
from after calling CipherUpdate, except for the setting of ctx->cont.
I made the attached change. It solves the particular problem case that
started my debugging. I will be using this code on lots more input, and
I'll report any problems.
I'd be very grateful if someone familiar with the code could look over
my change and suggest where else problems of this sort might be lurking.
I will certainly try to fix them if I think they could affect my code.
Andrew
--- bio_enc.orig Mon Dec 20 00:50:11 1999
+++ bio_enc.c Sat Jan 8 00:57:22 2000
@@ -182,11 +182,12 @@
(unsigned char *)ctx->buf,
&(ctx->buf_len));
ctx->ok=i;
- ctx->buf_off=0;
}
else
+ {
ret=(ret == 0)?i:ret;
- break;
+ break;
+ }
}
else
{
