Commit cca6198c authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

Avoid illegal read off end of an array in prune_v2_cipher_list 

This function is supposed to construct a list of all the ciphers in
the "v2 link protocol cipher list" that are supported by Tor's
openssl.  It does this by invoking ssl23_get_cipher_by_char on each
two-byte ciphersuite ID to see which ones give a match.  But when
ssl23_get_cipher_by_char cannot find a match for a two-byte SSL3/TLS
ciphersuite ID, it checks to see whether it has a match for a
three-byte SSL2 ciphersuite ID.  This was causing a read off the end
of the 'cipherid' array.

This was probably harmless in practice, but we shouldn't be having
any uninitialized reads.

(Using ssl23_get_cipher_by_char in this way is a kludge, but then
again the entire existence of the v2 link protocol is kind of a
kludge.  Once Tor 0.2.2 clients are all gone, we can drop this code
entirely.)

Found by starlight. Fix on 0.2.4.8-alpha. Fixes bug 12227.
parent 8d9602c2

changes/bug12227

0 → 100644
+5 −0
Original line number Diff line number Diff line 
  o Minor bugfixes:

    - Avoid an illegal read from stack when initializing the TLS

      module using a version of OpenSSL without all of the ciphers

      used by the v2 link handshake. Fixes bug 12227; bugfix on

      0.2.4.8-alpha.  Found by "starlight".

src/common/tortls.c

+4 −1
Original line number Diff line number Diff line
@@ -1489,10 +1489,13 @@ prune_v2_cipher_list(void) 


  inp = outp = v2_cipher_list;

  while (*inp) {

    unsigned char cipherid[2];

    unsigned char cipherid[3];

    const SSL_CIPHER *cipher;

    /* Is there no better way to do this? */

    set_uint16(cipherid, htons(*inp));

    cipherid[2] = 0; /* If ssl23_get_cipher_by_char finds no cipher starting

                      * with a two-byte 'cipherid', it may look for a v2

                      * cipher with the appropriate 3 bytes. */

    cipher = m->get_cipher_by_char(cipherid);

    if (cipher) {

      tor_assert((cipher->id & 0xffff) == *inp);