hi!
i would like to report some bugs in ssleay. unfortunately i don't have
diffs against latest openssl source, but the fixes are really small, so i
hope it's not too much trouble to incorporate them.
1) crypto/bio/b_printf.c uses static buffer for vsprintf which might
overflow. we should use vsnprintf. no source code to fix this bug :(
2) crypto/bio/bf_buff.c buffer_ctrl BIO_CTRL_FLUSH must flush the
underlying bios after write. (insert BIO_ctrl(b->next_bio,cmd,num,ptr);
before last break;)
3) crypto/bio/bf_buff.c buffer_gets does not insert final '\n' into
buffer. so it is not semanticaly equivalent with fgets. this is my version
of buffer_gets:
static int buffer_gets(b,buf,size)
BIO *b;
char *buf;
int size;
{
BIO_F_BUFFER_CTX *ctx;
int num=0,i;
char *p;
ctx=(BIO_F_BUFFER_CTX *)b->ptr;
size-= 2; /* leave room for '\n' and '\0' */
BIO_clear_retry_flags(b);
for (;;)
{
if (ctx->ibuf_len != 0)
{
p= &(ctx->ibuf[ctx->ibuf_off]);
for (i=0;i < ctx->ibuf_len && i < size && p[i] !=
'\n'; i++)
{
*(buf++)=p[i];
}
num+=i;
size-=i;
ctx->ibuf_len-=i;
ctx->ibuf_off+=i;
*buf= '\0';
if (p[i] == '\n') /* append linefeed */
{
*(buf++)='\n';
*buf='\0';
num++;
size--;
ctx->ibuf_len--;
ctx->ibuf_off++;
return(num);
}
else if( i == size ) /* output buffer full */
{
return(num);
}
}
else /* read another chunk */
{
i=BIO_read(b->next_bio,ctx->ibuf,ctx->ibuf_size);
if (i <= 0)
{
BIO_copy_next_retry(b);
if (i < 0) return((num > 0)?num:i);
if (i == 0) return(num);
}
ctx->ibuf_len=i;
ctx->ibuf_off=0;
}
}
}
4) crypto/bio/bss_sock.c BIO_sock_should_retry has following code:
#if defined(WINDOWS) /* more microsoft stupidity */
if ((i == -1) && (err == 0))
return(1);
#endif
i'm almost sure that this check is not needed. SSLeay 0.6.6 did not have
this check and it worked fine. in SSLeay 0.8.1 the function looked like
this:
#ifndef BIO_FD
int BIO_sock_should_retry(i)
#else
int BIO_fd_should_retry(i)
#endif
int i;
{
if ((i == 0) || (i == -1))
{
#if !defined(BIO_FD) && defined(WINDOWS)
errno=WSAGetLastError();
#endif
#if defined(WINDOWS) /* more microsoft stupidity */
if ((i == -1) && (errno == 0))
return(1);
#endif
#ifndef BIO_FD
return(BIO_sock_non_fatal_error(errno));
#else
return(BIO_fd_non_fatal_error(errno));
#endif
}
return(0);
}
using errno before WSAGetLastError is nasty bug. under Borland C for
example errno is defined as
#define errno (*__errno())
and __errno is function which allocates thread local variable for errno
and returns pointer to it. but the allocation of this variable is done via
system calls which reset the value returned by GetLastError (aka
WSAGetLastError) so the errno is always 0. and then they compensated this
by adding this check.
this check really hurts when i use non-blocking sockets under windows and
there is some non-io error during SSL handshake the handshake will never
finish. for example if i have a empty list of ciphers:
ssl3_connect in s3_pkt.c has at start WSASetLastError(0);
ssl3_client_hello creates list of ciphers, finds it is empty and returns
-1.
now i will call BIO_sock_should_retry(i) to determine if it is fatal error
or should i retry. but there is check in BIO_sock_should_retry which says
that i must retry even when the error is fatal.
5) crypto/err/err.c ERR_get_state has static variable fallback. this
should be initalized before returning pointer to it.
6) crypto/rsa/rsa_enc.c RSA_eay_mod_exp at the end:
- BN_CTX_free(ctx);
+ if (ctx != NULL ) BN_CTX_free(ctx);
7) crypto/x509/x509name.c X509_NAME_add_entry frees wrong name entry in
case of error:
err:
if (new_name != NULL)
- X509_NAME_ENTRY_free(ne);
+ X509_NAME_ENTRY_free(new_name);
return(0);
8) ssl/s2_pkt.c and ssl/s3_pkt.c write_pending and ssl3_write_pending have
unnecessary check at the beginning which stops me from moving data around
in my buffers between calls to SSL_write. this data is already copied to
internal buffers and there is no need for this check. i tested ssleay
without this check (non-blocking sockets and stuff) under various
platforms and everything worked as expected.
if ((s->s3->wpend_tot > (int)len) || (s->s3->wpend_buf != buf)
|| (s->s3->wpend_type != type))
{
SSLerr(SSL_F_SSL3_WRITE_PENDING,SSL_R_BAD_WRITE_RETRY);
return(-1);
}
i have couple of bigger addiotions to ssleay too: i added write capability
to conf module and rewrote it to use bio; bio_log module for logging to
syslog/event log and bio_reliable module for creating reliable streams. i
can send them directly to someone in core team for addition if you are
interested.
arne
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
