[Pkg-openssl-devel] Bug#681612: libssl1.0.0: fishy reliance on errno when errno is not set

[Marc Lehmann]

Package: libssl1.0.0
Version: 1.0.1-4
Severity: normal


Hi!

While investigating some unrelated issue, I had a look at openssl sources
(0.9.8, 1.0.0 and 1.0.1) and found some very questionable code that looks
like incorrect reliance on undefined errno values.

Specifically, what tipped me of is this macro:

   #define clear_sys_error()      errno=0

It is used a great number of times in openssl, so the problem I saw might
be endemic. The above macro cannot be of use in a POSIX (or GNU/Linux
program), except for very few functions that require it (none of which
openssl uses) or possibly to improve the debugging experience by having
errno 0 "more often" in the debugger.

However, openssl wrongly relies on it, at least in ./crypto/bio/bss_fd.c:

fd_read calls read(), and if read returns <= 0, it calls BIO_fd_should_retry.

BIO_fd_should_retry sets BIO_fd_non_fatal_error with errno if ret == 0.

However, when read returns 0, errno has undefined contents (0 is not a
failure, and errno is only defined after a failure), so openssl relies on
an undefined value in BIO_fd_non_fatal_error which in turn might make it
signal an error when none happened (or worse).

The intent is very likely to set errno to 0 before the call, and expect it
to be zero after the call if no error occurred, which is, however, only
true in "most cases" and not covered by any standards.

This issue is mitigated by the fact that the default read() on glibc on
GNU/Linux does not change errno. Other libcs, future glibc versions or
dynamically provided wrappers for read() need not behave so nicely though.

clear_sys_error is used 24 times or so in openssl sources - I have
only investigated the one above, but it is highly likely that openssl
relies on errno being unchanged on success in at least some other cases
as well, which should be investigated (specifically, while read() and
many direct syscalls don't change errno, many other functions in glibc
actually do leave non-zero values in errno in the success case, as this
is a common pattern when implementing libcs on unix-like operating
systems. Furthermore, on windows openssl uses GetLastError which has
effectively the same semantics as errno, although, of course, windows
shouldn't be of concern for Debian :).

(Actually, the glibc info documentation claims glibc does not change errno
on success, but that might be a typo, as the same paragraph warns you
that the proper way is to chekc errno only after a failure, and in the
next paragraph says that errno might be set when a library function calls
another library function internally. In any case, the manpages, POSIX and
actual behaviour are clear in that errno might be set to any value in the
success case).

Of course, I could also be blind/confused/whatever, but I did triple-check
the code, and it still looks as if openssl requires that errno is
unchanged after a sucessful read call, so I decided it's better to report
it so somebody who actually knows openssl can have a look at it.

Cheers,

-- System Information:
Debian Release: 6.0.3
  APT prefers stable
  APT policy: (990, 'stable'), (500, 'unstable'), (500, 'testing'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-3-amd64 (SMP w/4 CPU cores)
Locale: LANG=C, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages libssl1.0.0 depends on:
ii  debconf [debconf-2.0]   1.5.36.1         Debian configuration management sy
hi  libc6                   2.13-23          Embedded GNU C Library: Shared lib
ii  multiarch-support       2.13-24          Transitional package to ensure mul
ii  zlib1g                  1:1.2.3.4.dfsg-3 compression library - runtime

libssl1.0.0 recommends no packages.

libssl1.0.0 suggests no packages.

-- debconf information excluded