Skip to content
  1. May 22, 2015
    • Matt Caswell's avatar
      Fix off-by-one in BN_rand · b484b040
      Matt Caswell authored
      
      
      If BN_rand is called with |bits| set to 1 and |top| set to 1 then a 1 byte
      buffer overflow can occur. There are no such instances within the OpenSSL at
      the moment.
      
      Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke, Filip Palian for
      discovering and reporting this issue.
      
      Reviewed-by: default avatarKurt Roeckx <kurt@openssl.org>
      b484b040
    • Matt Caswell's avatar
      Reject negative shifts for BN_rshift and BN_lshift · 726b5e71
      Matt Caswell authored
      
      
      The functions BN_rshift and BN_lshift shift their arguments to the right or
      left by a specified number of bits. Unpredicatable results (including
      crashes) can occur if a negative number is supplied for the shift value.
      
      Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke and Filip Palian
      for discovering and reporting this issue.
      
      Reviewed-by: default avatarKurt Roeckx <kurt@openssl.org>
      (cherry picked from commit 7cc18d81)
      
      Conflicts:
      	crypto/bn/bn.h
      	crypto/bn/bn_err.c
      726b5e71
    • Lubom's avatar
      Lost alert in DTLS · 0a9f8e06
      Lubom authored
      
      
      If a client receives a bad hello request in DTLS then the alert is not
      sent correctly.
      
      RT#2801
      
      Signed-off-by: default avatarMatt Caswell <matt@openssl.org>
      Reviewed-by: default avatarKurt Roeckx <kurt@openssl.org>
      (cherry picked from commit 4dc1aa04)
      0a9f8e06
  2. May 20, 2015
  3. May 19, 2015
  4. May 15, 2015
  5. May 13, 2015
    • Rich Salz's avatar
      Add NULL checks from master · 76b49a8a
      Rich Salz authored
      
      
      The big "don't check for NULL" cleanup requires backporting some
      of the lowest-level functions to actually do nothing if NULL is
      given.  This will make it easier to backport fixes to release
      branches, where master assumes those lower-level functions are "safe"
      
      This commit addresses those tickets: 3798 3799 3801.
      
      Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
      (cherry picked from commit f34b095f)
      76b49a8a
    • Hanno Böck's avatar
      Call of memcmp with null pointers in obj_cmp() · 5e0ec901
      Hanno Böck authored
      
      
      The function obj_cmp() (file crypto/objects/obj_dat.c) can in some
      situations call memcmp() with a null pointer and a zero length.
      
      This is invalid behaviour. When compiling openssl with undefined
      behaviour sanitizer (add -fsanitize=undefined to compile flags) this
      can be seen. One example that triggers this behaviour is the pkcs7
      command (but there are others, e.g. I've seen it with the timestamp
      function):
      apps/openssl pkcs7 -in test/testp7.pem
      
      What happens is that obj_cmp takes objects of the type ASN1_OBJECT and
      passes their ->data pointer to memcmp. Zero-sized ASN1_OBJECT
      structures can have a null pointer as data.
      
      RT#3816
      
      Signed-off-by: default avatarMatt Caswell <matt@openssl.org>
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      (cherry picked from commit 2b8dc08b)
      5e0ec901
    • Matt Caswell's avatar
      Don't allow a CCS when expecting a CertificateVerify · 5c122908
      Matt Caswell authored
      
      
      Currently we set change_cipher_spec_ok to 1 before calling
      ssl3_get_cert_verify(). This is because this message is optional and if it
      is not sent then the next thing we would expect to get is the CCS. However,
      although it is optional, we do actually know whether we should be receiving
      one in advance. If we have received a client cert then we should expect
      a CertificateVerify message. By the time we get to this point we will
      already have bombed out if we didn't get a Certificate when we should have
      done, so it is safe just to check whether |peer| is NULL or not. If it is
      we won't get a CertificateVerify, otherwise we will. Therefore we should
      change the logic so that we only attempt to get the CertificateVerify if
      we are expecting one, and not allow a CCS in this scenario.
      
      Whilst this is good practice for TLS it is even more important for DTLS.
      In DTLS messages can be lost. Therefore we may be in a situation where a
      CertificateVerify message does not arrive even though one was sent. In that
      case the next message the server will receive will be the CCS. This could
      also happen if messages get re-ordered in-flight. In DTLS if
      |change_cipher_spec_ok| is not set and a CCS is received it is ignored.
      However if |change_cipher_spec_ok| *is* set then a CCS arrival will
      immediately move the server into the next epoch. Any messages arriving for
      the previous epoch will be ignored. This means that, in this scenario, the
      handshake can never complete. The client will attempt to retransmit
      missing messages, but the server will ignore them because they are the wrong
      epoch. The server meanwhile will still be waiting for the CertificateVerify
      which is never going to arrive.
      
      RT#2958
      
      Reviewed-by: default avatarEmilia Käsper <emilia@openssl.org>
      (cherry picked from commit a0bd6493)
      5c122908
  6. May 11, 2015
  7. May 05, 2015
  8. May 04, 2015
    • Richard Levitte's avatar
      RT2943: Check sizes if -iv and -K arguments · 4b771121
      Richard Levitte authored
      RT2943 only complains about the incorrect check of -K argument size,
      we might as well do the same thing with the -iv argument.
      
      Before this, we only checked that the given argument wouldn't give a
      bitstring larger than EVP_MAX_KEY_LENGTH.  we can be more precise and
      check against the size of the actual cipher used.
      
      (cherry picked from commit 8920a7cd
      
      )
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      4b771121
  9. May 02, 2015
  10. Apr 30, 2015
    • Matt Caswell's avatar
      Fix buffer overrun in RSA signing · 017f695f
      Matt Caswell authored
      
      
      The problem occurs in EVP_PKEY_sign() when using RSA with X931 padding.
      It is only triggered if the RSA key size is smaller than the digest length.
      So with SHA512 you can trigger the overflow with anything less than an RSA
      512 bit key. I managed to trigger a 62 byte overflow when using a 16 bit RSA
      key. This wasn't sufficient to cause a crash, although your mileage may
      vary.
      
      In practice RSA keys of this length are never used and X931 padding is very
      rare. Even if someone did use an excessively short RSA key, the chances of
      them combining that with a longer digest and X931 padding is very
      small. For these reasons I do not believe there is a security implication to
      this. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
      Solutions) for reporting this issue.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (cherry picked from commit 34166d41)
      017f695f
    • Matt Caswell's avatar
      Add sanity check to print_bin function · ee900ed1
      Matt Caswell authored
      
      
      Add a sanity check to the print_bin function to ensure that the |off|
      argument is positive. Thanks to Kevin Wojtysiak (Int3 Solutions) and
      Paramjot Oberoi (Int3 Solutions) for reporting this issue.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (cherry picked from commit 3deeeeb6)
      ee900ed1
    • Matt Caswell's avatar
      Add sanity check to ssl_get_prev_session · 39b36cb4
      Matt Caswell authored
      
      
      Sanity check the |len| parameter to ensure it is positive. Thanks to Kevin
      Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
      reporting this issue.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (cherry picked from commit cb0f400b)
      39b36cb4
    • Matt Caswell's avatar
      Sanity check the return from final_finish_mac · 26800340
      Matt Caswell authored
      
      
      The return value is checked for 0. This is currently safe but we should
      really check for <= 0 since -1 is frequently used for error conditions.
      Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
      Solutions) for reporting this issue.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (cherry picked from commit c427570e)
      
      Conflicts:
      	ssl/ssl_locl.h
      
      Conflicts:
      	ssl/ssl_locl.h
      26800340
    • Matt Caswell's avatar
      Add sanity check in ssl3_cbc_digest_record · 592ac253
      Matt Caswell authored
      
      
      For SSLv3 the code assumes that |header_length| > |md_block_size|. Whilst
      this is true for all SSLv3 ciphersuites, this fact is far from obvious by
      looking at the code. If this were not the case then an integer overflow
      would occur, leading to a subsequent buffer overflow. Therefore I have
      added an explicit sanity check to ensure header_length is always valid.
      Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
      Solutions) for reporting this issue.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (cherry picked from commit 29b0a15a)
      592ac253
    • Matt Caswell's avatar
      Clarify logic in BIO_*printf functions · d8896822
      Matt Caswell authored
      
      
      The static function dynamically allocates an output buffer if the output
      grows larger than the static buffer that is normally used. The original
      logic implied that |currlen| could be greater than |maxlen| which is
      incorrect (and if so would cause a buffer overrun). Also the original
      logic would call OPENSSL_malloc to create a dynamic buffer equal to the
      size of the static buffer, and then immediately call OPENSSL_realloc to
      make it bigger, rather than just creating a buffer than was big enough in
      the first place. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot
      Oberoi (Int3 Solutions) for reporting this issue.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (cherry picked from commit 9d9e3774)
      d8896822
    • Matt Caswell's avatar
      Sanity check EVP_EncodeUpdate buffer len · 951ede2a
      Matt Caswell authored
      
      
      There was already a sanity check to ensure the passed buffer length is not
      zero. Extend this to ensure that it also not negative. Thanks to Kevin
      Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
      reporting this issue.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (cherry picked from commit b86d7dca)
      951ede2a
    • Matt Caswell's avatar
      Sanity check EVP_CTRL_AEAD_TLS_AAD · 974d4d67
      Matt Caswell authored
      
      
      The various implementations of EVP_CTRL_AEAD_TLS_AAD expect a buffer of at
      least 13 bytes long. Add sanity checks to ensure that the length is at
      least that. Also add a new constant (EVP_AEAD_TLS1_AAD_LEN) to evp.h to
      represent this length. Thanks to Kevin Wojtysiak (Int3 Solutions) and
      Paramjot Oberoi (Int3 Solutions) for reporting this issue.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (cherry picked from commit c8269881)
      
      Conflicts:
      	ssl/record/ssl3_record.c
      
      Conflicts:
      	apps/speed.c
      	crypto/evp/e_aes_cbc_hmac_sha256.c
      	crypto/evp/evp.h
      974d4d67
    • Matt Caswell's avatar
      Sanity check DES_enc_write buffer length · 3be5df22
      Matt Caswell authored
      
      
      Add a sanity check to DES_enc_write to ensure the buffer length provided
      is not negative. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot
      Oberoi (Int3 Solutions) for reporting this issue.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (cherry picked from commit 873fb39f)
      3be5df22
  11. Apr 29, 2015
    • Matt Caswell's avatar
      Add length sanity check in SSLv2 n_do_ssl_write() · 80a06268
      Matt Caswell authored
      
      
      Fortify flagged up a problem in n_do_ssl_write() in SSLv2. Analysing the
      code I do not believe there is a real problem here. However the logic flows
      are complicated enough that a sanity check of |len| is probably worthwhile.
      
      Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
      Solutions) for reporting this issue.
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      (cherry picked from commit c5f8cd7b)
      80a06268
  12. Apr 22, 2015
    • Loganaden Velvindron's avatar
      Fix CRYPTO_strdup · e3dd33c2
      Loganaden Velvindron authored
      
      
      The function CRYPTO_strdup (aka OPENSSL_strdup) fails to check the return
      value from CRYPTO_malloc to see if it is NULL before attempting to use it.
      This patch adds a NULL check.
      
      RT3786
      
      Signed-off-by: default avatarMatt Caswell <matt@openssl.org>
      (cherry picked from commit 37b0cf936744d9edb99b5dd82cae78a7eac6ad60)
      
      Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
      (cherry picked from commit 20d21389c8b6f5b754573ffb6a4dc4f3986f2ca4)
      e3dd33c2