April 2017 - tor-commits - lists.torproject.org

[tor/master] Never read off the end of a buffer in base32_encode()
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 4812441d3465f4f2fc6763ee644f79d5a9c8661b Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 10:47:16 2017 -0400 Never read off the end of a buffer in base32_encode() When we "fixed" #18280 in 4e4a7d2b0c199227252a742541461ec4cc35d358 in 0291 it appears that we introduced a bug: The base32_encode function can read off the end of the input buffer, if the input buffer size modulo 5 is not equal to 0 or 3. This is not completely horrible, for two reasons: * The extra bits that are read are never actually used: so this is only a crash when asan is enabled, in the worst case. Not a data leak. * The input sizes passed to base32_encode are only ever multiples of 5. They are all either DIGEST_LEN (20), REND_SERVICE_ID_LEN (10), sizeof(rand_bytes) in addressmap.c (10), or an input in crypto.c that is forced to a multiple of 5. So this bug can't actually trigger in today's Tor. Closes bug 21894; bugfix on 0.2.9.1-alpha. --- changes/bug21894_029 | 5 +++++ src/common/util_format.c | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/changes/bug21894_029 b/changes/bug21894_029 new file mode 100644 index 0000000..e3a84fa --- /dev/null +++ b/changes/bug21894_029 @@ -0,0 +1,5 @@ + o Minor bugfixes (crash prevention): + - Fix an (currently untriggerable, but potentially dangerous) crash + bug when base32-encoding inputs whose sizes are not a multiple of + 5. Fixes bug 21894; bugfix on 0.2.9.1-alpha. + diff --git a/src/common/util_format.c b/src/common/util_format.c index aef9db8..a460842 100644 --- a/src/common/util_format.c +++ b/src/common/util_format.c @@ -51,9 +51,10 @@ base32_encode(char *dest, size_t destlen, const char *src, size_t srclen) for (i=0,bit=0; bit < nbits; ++i, bit+=5) { /* set v to the 16-bit value starting at src[bits/8], 0-padded. */ - v = ((uint8_t)src[bit/8]) << 8; - if (bit+5<nbits) - v += (uint8_t)src[(bit/8)+1]; + size_t idx = bit / 8; + v = ((uint8_t)src[idx]) << 8; + if (idx+1 < srclen) + v += (uint8_t)src[idx+1]; /* set u to the 5-bit value at the bit'th bit of buf. */ u = (v >> (11-(bit%8))) & 0x1F; dest[i] = BASE32_CHARS[u];

1 0

[tor/maint-0.3.0] Never read off the end of a buffer in base32_encode()
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 4812441d3465f4f2fc6763ee644f79d5a9c8661b Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 10:47:16 2017 -0400 Never read off the end of a buffer in base32_encode() When we "fixed" #18280 in 4e4a7d2b0c199227252a742541461ec4cc35d358 in 0291 it appears that we introduced a bug: The base32_encode function can read off the end of the input buffer, if the input buffer size modulo 5 is not equal to 0 or 3. This is not completely horrible, for two reasons: * The extra bits that are read are never actually used: so this is only a crash when asan is enabled, in the worst case. Not a data leak. * The input sizes passed to base32_encode are only ever multiples of 5. They are all either DIGEST_LEN (20), REND_SERVICE_ID_LEN (10), sizeof(rand_bytes) in addressmap.c (10), or an input in crypto.c that is forced to a multiple of 5. So this bug can't actually trigger in today's Tor. Closes bug 21894; bugfix on 0.2.9.1-alpha. --- changes/bug21894_029 | 5 +++++ src/common/util_format.c | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/changes/bug21894_029 b/changes/bug21894_029 new file mode 100644 index 0000000..e3a84fa --- /dev/null +++ b/changes/bug21894_029 @@ -0,0 +1,5 @@ + o Minor bugfixes (crash prevention): + - Fix an (currently untriggerable, but potentially dangerous) crash + bug when base32-encoding inputs whose sizes are not a multiple of + 5. Fixes bug 21894; bugfix on 0.2.9.1-alpha. + diff --git a/src/common/util_format.c b/src/common/util_format.c index aef9db8..a460842 100644 --- a/src/common/util_format.c +++ b/src/common/util_format.c @@ -51,9 +51,10 @@ base32_encode(char *dest, size_t destlen, const char *src, size_t srclen) for (i=0,bit=0; bit < nbits; ++i, bit+=5) { /* set v to the 16-bit value starting at src[bits/8], 0-padded. */ - v = ((uint8_t)src[bit/8]) << 8; - if (bit+5<nbits) - v += (uint8_t)src[(bit/8)+1]; + size_t idx = bit / 8; + v = ((uint8_t)src[idx]) << 8; + if (idx+1 < srclen) + v += (uint8_t)src[idx+1]; /* set u to the 5-bit value at the bit'th bit of buf. */ u = (v >> (11-(bit%8))) & 0x1F; dest[i] = BASE32_CHARS[u];

1 0

[tor/maint-0.3.0] Merge remote-tracking branch 'public/bug21894_029' into maint-0.3.0
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit f1613b53c57af05250b71f31eda037bf82f1e725 Merge: a5b50ef 4812441 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 14:03:25 2017 -0400 Merge remote-tracking branch 'public/bug21894_029' into maint-0.3.0 changes/bug21894_029 | 5 +++++ src/common/util_format.c | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-)

1 0

[tor/master] Merge branch 'maint-0.3.0'
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 05ef3b959d4d718e9ebb6e65e99383a4a5c8a8b2 Merge: f0fa7dc f1613b5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 14:03:40 2017 -0400 Merge branch 'maint-0.3.0' changes/bug21894_029 | 5 +++++ src/common/util_format.c | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-)

1 0

[tor/master] Merge remote-tracking branch 'public/bug21894_029' into maint-0.3.0
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit f1613b53c57af05250b71f31eda037bf82f1e725 Merge: a5b50ef 4812441 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 14:03:25 2017 -0400 Merge remote-tracking branch 'public/bug21894_029' into maint-0.3.0 changes/bug21894_029 | 5 +++++ src/common/util_format.c | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-)

1 0

[tor/release-0.3.0] Never read off the end of a buffer in base32_encode()
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 4812441d3465f4f2fc6763ee644f79d5a9c8661b Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 10:47:16 2017 -0400 Never read off the end of a buffer in base32_encode() When we "fixed" #18280 in 4e4a7d2b0c199227252a742541461ec4cc35d358 in 0291 it appears that we introduced a bug: The base32_encode function can read off the end of the input buffer, if the input buffer size modulo 5 is not equal to 0 or 3. This is not completely horrible, for two reasons: * The extra bits that are read are never actually used: so this is only a crash when asan is enabled, in the worst case. Not a data leak. * The input sizes passed to base32_encode are only ever multiples of 5. They are all either DIGEST_LEN (20), REND_SERVICE_ID_LEN (10), sizeof(rand_bytes) in addressmap.c (10), or an input in crypto.c that is forced to a multiple of 5. So this bug can't actually trigger in today's Tor. Closes bug 21894; bugfix on 0.2.9.1-alpha. --- changes/bug21894_029 | 5 +++++ src/common/util_format.c | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/changes/bug21894_029 b/changes/bug21894_029 new file mode 100644 index 0000000..e3a84fa --- /dev/null +++ b/changes/bug21894_029 @@ -0,0 +1,5 @@ + o Minor bugfixes (crash prevention): + - Fix an (currently untriggerable, but potentially dangerous) crash + bug when base32-encoding inputs whose sizes are not a multiple of + 5. Fixes bug 21894; bugfix on 0.2.9.1-alpha. + diff --git a/src/common/util_format.c b/src/common/util_format.c index aef9db8..a460842 100644 --- a/src/common/util_format.c +++ b/src/common/util_format.c @@ -51,9 +51,10 @@ base32_encode(char *dest, size_t destlen, const char *src, size_t srclen) for (i=0,bit=0; bit < nbits; ++i, bit+=5) { /* set v to the 16-bit value starting at src[bits/8], 0-padded. */ - v = ((uint8_t)src[bit/8]) << 8; - if (bit+5<nbits) - v += (uint8_t)src[(bit/8)+1]; + size_t idx = bit / 8; + v = ((uint8_t)src[idx]) << 8; + if (idx+1 < srclen) + v += (uint8_t)src[idx+1]; /* set u to the 5-bit value at the bit'th bit of buf. */ u = (v >> (11-(bit%8))) & 0x1F; dest[i] = BASE32_CHARS[u];

1 0

[tor/release-0.3.0] Merge remote-tracking branch 'public/bug21894_029' into maint-0.3.0
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit f1613b53c57af05250b71f31eda037bf82f1e725 Merge: a5b50ef 4812441 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 14:03:25 2017 -0400 Merge remote-tracking branch 'public/bug21894_029' into maint-0.3.0 changes/bug21894_029 | 5 +++++ src/common/util_format.c | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-)

1 0

[tor/release-0.3.0] Merge branch 'maint-0.3.0' into release-0.3.0
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 03e521670059c1b29f8b0068b8fe86700183b884 Merge: e9a315c f1613b5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 14:03:37 2017 -0400 Merge branch 'maint-0.3.0' into release-0.3.0 changes/bug21894_029 | 5 +++++ src/common/util_format.c | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-)

1 0

[tor/master] Remove tor-checkkey as obsolete
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 7b60f0129a12543cf08a2cef5c13b8bfa6691459 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 31 10:57:48 2017 -0400 Remove tor-checkkey as obsolete CVE-2008-0166 is long gone, and we no longer need a helper tool to dump out public key moduli so folks can detect it. Closes ticket 21842. --- changes/ticket21842 | 6 ++++ src/common/crypto.c | 2 +- src/tools/include.am | 11 ------ src/tools/tor-checkkey.c | 89 ------------------------------------------------ 4 files changed, 7 insertions(+), 101 deletions(-) diff --git a/changes/ticket21842 b/changes/ticket21842 new file mode 100644 index 0000000..4b039c6 --- /dev/null +++ b/changes/ticket21842 @@ -0,0 +1,6 @@ + o Removed features: + - We've removed the tor-checkkey tool from src/tools. Long ago, we + used it to help people detect RSA keys that were generated by + versions of Debian affected by CVE-2008-0166. But those keys + have been out of circulation for ages, and this tool is no + longer required. Closes ticket 21842. diff --git a/src/common/crypto.c b/src/common/crypto.c index a5eb7b5..6519719 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -467,7 +467,7 @@ crypto_new_pk_from_rsa_(RSA *rsa) return env; } -/** Helper, used by tor-checkkey.c and tor-gencert.c. Return the RSA from a +/** Helper, used by tor-gencert.c. Return the RSA from a * crypto_pk_t. */ RSA * crypto_pk_get_rsa_(crypto_pk_t *env) diff --git a/src/tools/include.am b/src/tools/include.am index d0185b5..5eadb03 100644 --- a/src/tools/include.am +++ b/src/tools/include.am @@ -1,5 +1,4 @@ bin_PROGRAMS+= src/tools/tor-resolve src/tools/tor-gencert -noinst_PROGRAMS+= src/tools/tor-checkkey if COVERAGE_ENABLED noinst_PROGRAMS+= src/tools/tor-cov-resolve src/tools/tor-cov-gencert @@ -43,14 +42,4 @@ src_tools_tor_cov_gencert_LDADD = src/common/libor-testing.a \ @TOR_LIB_WS32@ @TOR_LIB_GDI@ @CURVE25519_LIBS@ endif -src_tools_tor_checkkey_SOURCES = src/tools/tor-checkkey.c -src_tools_tor_checkkey_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ -src_tools_tor_checkkey_LDADD = src/common/libor.a \ - src/common/libor-ctime.a \ - src/common/libor-crypto.a \ - $(LIBKECCAK_TINY) \ - $(LIBDONNA) \ - @TOR_LIB_MATH@ @TOR_ZLIB_LIBS@ @TOR_OPENSSL_LIBS@ \ - @TOR_LIB_WS32@ @TOR_LIB_GDI@ @CURVE25519_LIBS@ - EXTRA_DIST += src/tools/tor-fw-helper/README diff --git a/src/tools/tor-checkkey.c b/src/tools/tor-checkkey.c deleted file mode 100644 index f2f709d..0000000 --- a/src/tools/tor-checkkey.c +++ /dev/null @@ -1,89 +0,0 @@ -/* Copyright (c) 2008-2017, The Tor Project, Inc. */ -/* See LICENSE for licensing information */ - -#include "orconfig.h" - -#include <stdio.h> -#include <stdlib.h> -#include "crypto.h" -#include "torlog.h" -#include "util.h" -#include "compat.h" -#include "compat_openssl.h" -#include <openssl/bn.h> -#include <openssl/rsa.h> - -int -main(int c, char **v) -{ - crypto_pk_t *env; - char *str; - RSA *rsa; - int wantdigest=0; - int fname_idx; - char *fname=NULL; - init_logging(1); - - if (c < 2) { - fprintf(stderr, "Hi. I'm tor-checkkey. Tell me a filename that " - "has a PEM-encoded RSA public key (like in a cert) and I'll " - "dump the modulus. Use the --digest option too and I'll " - "dump the digest.\n"); - return 1; - } - - if (crypto_global_init(0, NULL, NULL)) { - fprintf(stderr, "Couldn't initialize crypto library.\n"); - return 1; - } - - if (!strcmp(v[1], "--digest")) { - wantdigest = 1; - fname_idx = 2; - if (c<3) { - fprintf(stderr, "too few arguments"); - return 1; - } - } else { - wantdigest = 0; - fname_idx = 1; - } - - fname = expand_filename(v[fname_idx]); - str = read_file_to_str(fname, 0, NULL); - tor_free(fname); - if (!str) { - fprintf(stderr, "Couldn't read %s\n", v[fname_idx]); - return 1; - } - - env = crypto_pk_new(); - if (crypto_pk_read_public_key_from_string(env, str, strlen(str))<0) { - fprintf(stderr, "Couldn't parse key.\n"); - return 1; - } - tor_free(str); - - if (wantdigest) { - char digest[HEX_DIGEST_LEN+1]; - if (crypto_pk_get_fingerprint(env, digest, 0)<0) - return 1; - printf("%s\n",digest); - } else { - rsa = crypto_pk_get_rsa_(env); - - const BIGNUM *rsa_n; -#ifdef OPENSSL_1_1_API - const BIGNUM *rsa_e, *rsa_d; - RSA_get0_key(rsa, &rsa_n, &rsa_e, &rsa_d); -#else - rsa_n = rsa->n; -#endif - str = BN_bn2hex(rsa_n); - - printf("%s\n", str); - } - - return 0; -} -

1 0

[tor/master] Merge branch 'ticket21842_squashed'
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit f0fa7dcdf048b418640d2e624e5b111ed1956214 Merge: 506b4bf 7b60f01 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 13:21:15 2017 -0400 Merge branch 'ticket21842_squashed' changes/ticket21842 | 6 ++++ src/common/crypto.c | 2 +- src/tools/include.am | 11 ------ src/tools/tor-checkkey.c | 89 ------------------------------------------------ 4 files changed, 7 insertions(+), 101 deletions(-)

1 0