[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[or-cvs] [tor/master] Merge branch 'bug2352_obsize' into maint-0.2.1



commit b97b0efec81c5564999c2545dd7f0ca230b239cc
Merge: 31b562e 1f3b442
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date:   Sat Jan 15 13:15:06 2011 -0500

    Merge branch 'bug2352_obsize' into maint-0.2.1

 changes/bug2352      |    6 ++++++
 src/common/crypto.c  |   14 ++++++++------
 src/common/crypto.h  |    2 +-
 src/or/routerparse.c |   16 ++++++++++++----
 src/or/test.c        |    6 +++---
 5 files changed, 30 insertions(+), 14 deletions(-)

diff --combined src/common/crypto.c
index 7cb849a,da4e03e..208e1c5
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@@ -439,21 -439,23 +439,23 @@@ crypto_pk_generate_key(crypto_pk_env_t 
    return 0;
  }
  
- /** Read a PEM-encoded private key from the string <b>s</b> into <b>env</b>.
-  * Return 0 on success, -1 on failure.
+ /** Read a PEM-encoded private key from the <b>len</b>-byte string <b>s</b>
+  * into <b>env</b>.  Return 0 on success, -1 on failure.  If len is -1,
+  * the string is nul-terminated.
   */
  /* Used here, and used for testing. */
  int
  crypto_pk_read_private_key_from_string(crypto_pk_env_t *env,
-                                        const char *s)
+                                        const char *s, ssize_t len)
  {
    BIO *b;
  
    tor_assert(env);
    tor_assert(s);
+   tor_assert(len < INT_MAX && len < SIZE_T_CEILING);
  
-   /* Create a read-only memory BIO, backed by the NUL-terminated string 's' */
-   b = BIO_new_mem_buf((char*)s, -1);
+   /* Create a read-only memory BIO, backed by the string 's' */
+   b = BIO_new_mem_buf((char*)s, (int)len);
  
    if (env->key)
      RSA_free(env->key);
@@@ -487,7 -489,7 +489,7 @@@ crypto_pk_read_private_key_from_filenam
    }
  
    /* Try to parse it. */
-   r = crypto_pk_read_private_key_from_string(env, contents);
+   r = crypto_pk_read_private_key_from_string(env, contents, -1);
    tor_free(contents);
    if (r)
      return -1; /* read_private_key_from_string already warned, so we don't.*/
@@@ -717,12 -719,9 +719,12 @@@ crypto_pk_copy_full(crypto_pk_env_t *en
   * in <b>env</b>, using the padding method <b>padding</b>.  On success,
   * write the result to <b>to</b>, and return the number of bytes
   * written.  On failure, return -1.
 + *
 + * <b>tolen</b> is the number of writable bytes in <b>to</b>, and must be
 + * at least the length of the modulus of <b>env</b>.
   */
  int
 -crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to,
 +crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to, size_t tolen,
                           const char *from, size_t fromlen, int padding)
  {
    int r;
@@@ -730,7 -729,6 +732,7 @@@
    tor_assert(from);
    tor_assert(to);
    tor_assert(fromlen<INT_MAX);
 +  tor_assert(tolen >= crypto_pk_keysize(env));
  
    r = RSA_public_encrypt((int)fromlen,
                           (unsigned char*)from, (unsigned char*)to,
@@@ -746,13 -744,9 +748,13 @@@
   * in <b>env</b>, using the padding method <b>padding</b>.  On success,
   * write the result to <b>to</b>, and return the number of bytes
   * written.  On failure, return -1.
 + *
 + * <b>tolen</b> is the number of writable bytes in <b>to</b>, and must be
 + * at least the length of the modulus of <b>env</b>.
   */
  int
  crypto_pk_private_decrypt(crypto_pk_env_t *env, char *to,
 +                          size_t tolen,
                            const char *from, size_t fromlen,
                            int padding, int warnOnFailure)
  {
@@@ -762,7 -756,6 +764,7 @@@
    tor_assert(to);
    tor_assert(env->key);
    tor_assert(fromlen<INT_MAX);
 +  tor_assert(tolen >= crypto_pk_keysize(env));
    if (!env->key->p)
      /* Not a private key */
      return -1;
@@@ -783,13 -776,9 +785,13 @@@
   * public key in <b>env</b>, using PKCS1 padding.  On success, write the
   * signed data to <b>to</b>, and return the number of bytes written.
   * On failure, return -1.
 + *
 + * <b>tolen</b> is the number of writable bytes in <b>to</b>, and must be
 + * at least the length of the modulus of <b>env</b>.
   */
  int
  crypto_pk_public_checksig(crypto_pk_env_t *env, char *to,
 +                          size_t tolen,
                            const char *from, size_t fromlen)
  {
    int r;
@@@ -797,7 -786,6 +799,7 @@@
    tor_assert(from);
    tor_assert(to);
    tor_assert(fromlen < INT_MAX);
 +  tor_assert(tolen >= crypto_pk_keysize(env));
    r = RSA_public_decrypt((int)fromlen,
                           (unsigned char*)from, (unsigned char*)to,
                           env->key, RSA_PKCS1_PADDING);
@@@ -820,7 -808,6 +822,7 @@@ crypto_pk_public_checksig_digest(crypto
  {
    char digest[DIGEST_LEN];
    char *buf;
 +  size_t buflen;
    int r;
  
    tor_assert(env);
@@@ -833,9 -820,8 +835,9 @@@
      log_warn(LD_BUG, "couldn't compute digest");
      return -1;
    }
 -  buf = tor_malloc(crypto_pk_keysize(env)+1);
 -  r = crypto_pk_public_checksig(env,buf,sig,siglen);
 +  buflen = crypto_pk_keysize(env)+1;
 +  buf = tor_malloc(buflen);
 +  r = crypto_pk_public_checksig(env,buf,buflen,sig,siglen);
    if (r != DIGEST_LEN) {
      log_warn(LD_CRYPTO, "Invalid signature");
      tor_free(buf);
@@@ -855,12 -841,9 +857,12 @@@
   * <b>env</b>, using PKCS1 padding.  On success, write the signature to
   * <b>to</b>, and return the number of bytes written.  On failure, return
   * -1.
 + *
 + * <b>tolen</b> is the number of writable bytes in <b>to</b>, and must be
 + * at least the length of the modulus of <b>env</b>.
   */
  int
 -crypto_pk_private_sign(crypto_pk_env_t *env, char *to,
 +crypto_pk_private_sign(crypto_pk_env_t *env, char *to, size_t tolen,
                         const char *from, size_t fromlen)
  {
    int r;
@@@ -868,7 -851,6 +870,7 @@@
    tor_assert(from);
    tor_assert(to);
    tor_assert(fromlen < INT_MAX);
 +  tor_assert(tolen >= crypto_pk_keysize(env));
    if (!env->key->p)
      /* Not a private key */
      return -1;
@@@ -887,19 -869,16 +889,19 @@@
   * <b>from</b>; sign the data with the private key in <b>env</b>, and
   * store it in <b>to</b>.  Return the number of bytes written on
   * success, and -1 on failure.
 + *
 + * <b>tolen</b> is the number of writable bytes in <b>to</b>, and must be
 + * at least the length of the modulus of <b>env</b>.
   */
  int
 -crypto_pk_private_sign_digest(crypto_pk_env_t *env, char *to,
 +crypto_pk_private_sign_digest(crypto_pk_env_t *env, char *to, size_t tolen,
                                const char *from, size_t fromlen)
  {
    int r;
    char digest[DIGEST_LEN];
    if (crypto_digest(digest,from,fromlen)<0)
      return -1;
 -  r = crypto_pk_private_sign(env,to,digest,DIGEST_LEN);
 +  r = crypto_pk_private_sign(env,to,tolen,digest,DIGEST_LEN);
    memset(digest, 0, sizeof(digest));
    return r;
  }
@@@ -923,7 -902,7 +925,7 @@@
   */
  int
  crypto_pk_public_hybrid_encrypt(crypto_pk_env_t *env,
 -                                char *to,
 +                                char *to, size_t tolen,
                                  const char *from,
                                  size_t fromlen,
                                  int padding, int force)
@@@ -946,13 -925,8 +948,13 @@@
  
    if (!force && fromlen+overhead <= pkeylen) {
      /* It all fits in a single encrypt. */
 -    return crypto_pk_public_encrypt(env,to,from,fromlen,padding);
 +    return crypto_pk_public_encrypt(env,to,
 +                                    tolen,
 +                                    from,fromlen,padding);
    }
 +  tor_assert(tolen >= fromlen + overhead + CIPHER_KEY_LEN);
 +  tor_assert(tolen >= pkeylen);
 +
    cipher = crypto_new_cipher_env();
    if (!cipher) return -1;
    if (crypto_cipher_generate_key(cipher)<0)
@@@ -974,7 -948,7 +976,7 @@@
    /* Length of symmetrically encrypted data. */
    symlen = fromlen-(pkeylen-overhead-CIPHER_KEY_LEN);
  
 -  outlen = crypto_pk_public_encrypt(env,to,buf,pkeylen-overhead,padding);
 +  outlen = crypto_pk_public_encrypt(env,to,tolen,buf,pkeylen-overhead,padding);
    if (outlen!=(int)pkeylen) {
      goto err;
    }
@@@ -1000,7 -974,6 +1002,7 @@@
  int
  crypto_pk_private_hybrid_decrypt(crypto_pk_env_t *env,
                                   char *to,
 +                                 size_t tolen,
                                   const char *from,
                                   size_t fromlen,
                                   int padding, int warnOnFailure)
@@@ -1014,12 -987,11 +1016,12 @@@
    pkeylen = crypto_pk_keysize(env);
  
    if (fromlen <= pkeylen) {
 -    return crypto_pk_private_decrypt(env,to,from,fromlen,padding,
 +    return crypto_pk_private_decrypt(env,to,tolen,from,fromlen,padding,
                                       warnOnFailure);
    }
 +
    buf = tor_malloc(pkeylen+1);
 -  outlen = crypto_pk_private_decrypt(env,buf,from,pkeylen,padding,
 +  outlen = crypto_pk_private_decrypt(env,buf,pkeylen+1,from,pkeylen,padding,
                                       warnOnFailure);
    if (outlen<0) {
      log_fn(warnOnFailure?LOG_WARN:LOG_DEBUG, LD_CRYPTO,
@@@ -1037,7 -1009,6 +1039,7 @@@
    }
    memcpy(to,buf+CIPHER_KEY_LEN,outlen-CIPHER_KEY_LEN);
    outlen -= CIPHER_KEY_LEN;
 +  tor_assert(tolen - outlen >= fromlen - pkeylen);
    r = crypto_cipher_decrypt(cipher, to+outlen, from+pkeylen, fromlen-pkeylen);
    if (r<0)
      goto err;
diff --combined src/common/crypto.h
index 9cfb414,713a988..d6f5555
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@@ -82,7 -82,7 +82,7 @@@ int crypto_pk_write_private_key_to_stri
  int crypto_pk_read_public_key_from_string(crypto_pk_env_t *env,
                                            const char *src, size_t len);
  int crypto_pk_read_private_key_from_string(crypto_pk_env_t *env,
-                                            const char *s);
+                                            const char *s, ssize_t len);
  int crypto_pk_write_private_key_to_filename(crypto_pk_env_t *env,
                                              const char *fname);
  
@@@ -93,25 -93,23 +93,25 @@@ crypto_pk_env_t *crypto_pk_dup_key(cryp
  crypto_pk_env_t *crypto_pk_copy_full(crypto_pk_env_t *orig);
  int crypto_pk_key_is_private(const crypto_pk_env_t *key);
  
 -int crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to,
 +int crypto_pk_public_encrypt(crypto_pk_env_t *env, char *to, size_t tolen,
                               const char *from, size_t fromlen, int padding);
 -int crypto_pk_private_decrypt(crypto_pk_env_t *env, char *to,
 +int crypto_pk_private_decrypt(crypto_pk_env_t *env, char *to, size_t tolen,
                                const char *from, size_t fromlen,
                                int padding, int warnOnFailure);
 -int crypto_pk_public_checksig(crypto_pk_env_t *env, char *to,
 +int crypto_pk_public_checksig(crypto_pk_env_t *env, char *to, size_t tolen,
                                const char *from, size_t fromlen);
  int crypto_pk_public_checksig_digest(crypto_pk_env_t *env, const char *data,
                                 size_t datalen, const char *sig, size_t siglen);
 -int crypto_pk_private_sign(crypto_pk_env_t *env, char *to,
 +int crypto_pk_private_sign(crypto_pk_env_t *env, char *to, size_t tolen,
                             const char *from, size_t fromlen);
 -int crypto_pk_private_sign_digest(crypto_pk_env_t *env, char *to,
 +int crypto_pk_private_sign_digest(crypto_pk_env_t *env, char *to, size_t tolen,
                                    const char *from, size_t fromlen);
  int crypto_pk_public_hybrid_encrypt(crypto_pk_env_t *env, char *to,
 +                                    size_t tolen,
                                      const char *from, size_t fromlen,
                                      int padding, int force);
  int crypto_pk_private_hybrid_decrypt(crypto_pk_env_t *env, char *to,
 +                                     size_t tolen,
                                       const char *from, size_t fromlen,
                                       int padding, int warnOnFailure);
  
diff --combined src/or/routerparse.c
index 9ad84ed,3aaefec..3778509
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@@ -571,12 -571,10 +571,12 @@@ router_append_dirobj_signature(char *bu
                                 crypto_pk_env_t *private_key)
  {
    char *signature;
 -  size_t i;
 +  size_t i, keysize;
  
 -  signature = tor_malloc(crypto_pk_keysize(private_key));
 -  if (crypto_pk_private_sign(private_key, signature, digest, DIGEST_LEN) < 0) {
 +  keysize = crypto_pk_keysize(private_key);
 +  signature = tor_malloc(keysize);
 +  if (crypto_pk_private_sign(private_key, signature, keysize,
 +                             digest, DIGEST_LEN) < 0) {
  
      log_warn(LD_BUG,"Couldn't sign digest.");
      goto err;
@@@ -926,7 -924,6 +926,7 @@@ check_signature_token(const char *diges
                        const char *doctype)
  {
    char *signed_digest;
 +  size_t keysize;
    const int check_authority = (flags & CST_CHECK_AUTHORITY);
    const int check_objtype = ! (flags & CST_NO_CHECK_OBJTYPE);
  
@@@ -948,10 -945,9 +948,10 @@@
      }
    }
  
 -  signed_digest = tor_malloc(tok->object_size);
 -  if (crypto_pk_public_checksig(pkey, signed_digest, tok->object_body,
 -                                tok->object_size)
 +  keysize = crypto_pk_keysize(pkey);
 +  signed_digest = tor_malloc(keysize);
 +  if (crypto_pk_public_checksig(pkey, signed_digest, keysize,
 +                                tok->object_body, tok->object_size)
        != DIGEST_LEN) {
      log_warn(LD_DIR, "Error reading %s: invalid signature.", doctype);
      tor_free(signed_digest);
@@@ -2553,7 -2549,7 +2553,7 @@@ networkstatus_parse_vote_from_string(co
          goto err;
        v->good_signature = 1;
      } else {
-       if (tok->object_size >= INT_MAX)
+       if (tok->object_size >= INT_MAX || tok->object_size >= SIZE_T_CEILING)
          goto err;
        /* We already parsed a vote from this voter. Use the first one. */
        if (v->signature) {
@@@ -2704,7 -2700,7 +2704,7 @@@ networkstatus_parse_detached_signatures
        voter = tor_malloc_zero(sizeof(networkstatus_voter_info_t));
        memcpy(voter->identity_digest, id_digest, DIGEST_LEN);
        memcpy(voter->signing_key_digest, sk_digest, DIGEST_LEN);
-       if (tok->object_size >= INT_MAX)
+       if (tok->object_size >= INT_MAX || tok->object_size >= SIZE_T_CEILING)
          goto err;
        voter->signature = tor_memdup(tok->object_body, tok->object_size);
        voter->signature_len = (int) tok->object_size;
@@@ -3021,6 -3017,10 +3021,10 @@@ static directory_token_t 
  get_next_token(memarea_t *area,
                 const char **s, const char *eos, token_rule_t *table)
  {
+   /** Reject any object at least this big; it is probably an overflow, an
+    * attack, a bug, or some other nonsense. */
+ #define MAX_UNPARSED_OBJECT_SIZE (128*1024)
+ 
    const char *next, *eol, *obstart;
    size_t obname_len;
    int i;
@@@ -3105,7 -3105,8 +3109,8 @@@
  
    obstart = *s; /* Set obstart to start of object spec */
    if (*s+16 >= eol || memchr(*s+11,'\0',eol-*s-16) || /* no short lines, */
-       strcmp_len(eol-5, "-----", 5)) {          /* nuls or invalid endings */
+       strcmp_len(eol-5, "-----", 5) ||           /* nuls or invalid endings */
+       (eol-*s) > MAX_UNPARSED_OBJECT_SIZE) {     /* name too long */
      RET_ERR("Malformed object: bad begin line");
    }
    tok->object_type = STRNDUP(*s+11, eol-*s-16);
@@@ -3130,13 -3131,16 +3135,16 @@@
      ebuf[sizeof(ebuf)-1] = '\0';
      RET_ERR(ebuf);
    }
+   if (next - *s > MAX_UNPARSED_OBJECT_SIZE)
+     RET_ERR("Couldn't parse object: missing footer or object much too big.");
+ 
    if (!strcmp(tok->object_type, "RSA PUBLIC KEY")) { /* If it's a public key */
      tok->key = crypto_new_pk_env();
      if (crypto_pk_read_public_key_from_string(tok->key, obstart, eol-obstart))
        RET_ERR("Couldn't parse public key.");
    } else if (!strcmp(tok->object_type, "RSA PRIVATE KEY")) { /* private key */
      tok->key = crypto_new_pk_env();
-     if (crypto_pk_read_private_key_from_string(tok->key, obstart))
+     if (crypto_pk_read_private_key_from_string(tok->key, obstart, eol-obstart))
        RET_ERR("Couldn't parse private key.");
    } else { /* If it's something else, try to base64-decode it */
      int r;
diff --combined src/or/test.c
index 103866b,c4458d8..904ca69
--- a/src/or/test.c
+++ b/src/or/test.c
@@@ -701,27 -701,25 +701,27 @@@ test_crypto_pk(void
    test_eq(128, crypto_pk_keysize(pk1));
    test_eq(128, crypto_pk_keysize(pk2));
  
 -  test_eq(128, crypto_pk_public_encrypt(pk2, data1, "Hello whirled.", 15,
 +  test_eq(128, crypto_pk_public_encrypt(pk2, data1, sizeof(data1),
 +                                        "Hello whirled.", 15,
                                          PK_PKCS1_OAEP_PADDING));
 -  test_eq(128, crypto_pk_public_encrypt(pk1, data2, "Hello whirled.", 15,
 +  test_eq(128, crypto_pk_public_encrypt(pk1, data2, sizeof(data2),
 +                                        "Hello whirled.", 15,
                                          PK_PKCS1_OAEP_PADDING));
    /* oaep padding should make encryption not match */
    test_memneq(data1, data2, 128);
 -  test_eq(15, crypto_pk_private_decrypt(pk1, data3, data1, 128,
 +  test_eq(15, crypto_pk_private_decrypt(pk1, data3, sizeof(data3), data1, 128,
                                          PK_PKCS1_OAEP_PADDING,1));
    test_streq(data3, "Hello whirled.");
    memset(data3, 0, 1024);
 -  test_eq(15, crypto_pk_private_decrypt(pk1, data3, data2, 128,
 +  test_eq(15, crypto_pk_private_decrypt(pk1, data3, sizeof(data3), data2, 128,
                                          PK_PKCS1_OAEP_PADDING,1));
    test_streq(data3, "Hello whirled.");
    /* Can't decrypt with public key. */
 -  test_eq(-1, crypto_pk_private_decrypt(pk2, data3, data2, 128,
 +  test_eq(-1, crypto_pk_private_decrypt(pk2, data3, sizeof(data3), data2, 128,
                                          PK_PKCS1_OAEP_PADDING,1));
    /* Try again with bad padding */
    memcpy(data2+1, "XYZZY", 5);  /* This has fails ~ once-in-2^40 */
 -  test_eq(-1, crypto_pk_private_decrypt(pk1, data3, data2, 128,
 +  test_eq(-1, crypto_pk_private_decrypt(pk1, data3, sizeof(data3), data2, 128,
                                          PK_PKCS1_OAEP_PADDING,1));
  
    /* File operations: save and load private key */
@@@ -736,22 -734,19 +736,22 @@@
                                                     get_fname("xyzzy")) < 0);
    test_assert(! crypto_pk_read_private_key_from_filename(pk2,
                                                           get_fname("pkey1")));
 -  test_eq(15, crypto_pk_private_decrypt(pk2, data3, data1, 128,
 +  test_eq(15, crypto_pk_private_decrypt(pk2, data3, sizeof(data3), data1, 128,
                                          PK_PKCS1_OAEP_PADDING,1));
  
    /* Now try signing. */
    strlcpy(data1, "Ossifrage", 1024);
 -  test_eq(128, crypto_pk_private_sign(pk1, data2, data1, 10));
 -  test_eq(10, crypto_pk_public_checksig(pk1, data3, data2, 128));
 +  test_eq(128, crypto_pk_private_sign(pk1, data2, sizeof(data2), data1, 10));
 +  test_eq(10, crypto_pk_public_checksig(pk1, data3, sizeof(data3), data2, 128));
    test_streq(data3, "Ossifrage");
    /* Try signing digests. */
 -  test_eq(128, crypto_pk_private_sign_digest(pk1, data2, data1, 10));
 -  test_eq(20, crypto_pk_public_checksig(pk1, data3, data2, 128));
 -  test_eq(0, crypto_pk_public_checksig_digest(pk1, data1, 10, data2, 128));
 -  test_eq(-1, crypto_pk_public_checksig_digest(pk1, data1, 11, data2, 128));
 +  test_eq(128, crypto_pk_private_sign_digest(pk1, data2, sizeof(data2),
 +                                             data1, 10));
 +  test_eq(20, crypto_pk_public_checksig(pk1, data3, sizeof(data1), data2, 128));
 +  test_eq(0, crypto_pk_public_checksig_digest(pk1, data1,
 +                                              10, data2, 128));
 +  test_eq(-1, crypto_pk_public_checksig_digest(pk1, data1,
 +                                               11, data2, 128));
    /*XXXX test failed signing*/
  
    /* Try encoding */
@@@ -772,11 -767,9 +772,11 @@@
          continue;
        p = (i==0)?PK_NO_PADDING:
          (i==1)?PK_PKCS1_PADDING:PK_PKCS1_OAEP_PADDING;
 -      len = crypto_pk_public_hybrid_encrypt(pk1,data2,data1,j,p,0);
 +      len = crypto_pk_public_hybrid_encrypt(pk1,data2,sizeof(data2),
 +                                            data1,j,p,0);
        test_assert(len>=0);
 -      len = crypto_pk_private_hybrid_decrypt(pk1,data3,data2,len,p,1);
 +      len = crypto_pk_private_hybrid_decrypt(pk1,data3,sizeof(data3),
 +                                             data2,len,p,1);
        test_eq(len,j);
        test_memeq(data1,data3,j);
      }
@@@ -3368,11 -3361,11 +3368,11 @@@ test_v3_networkstatus(void
    sign_skey_leg1 = pk_generate(4);
  
    test_assert(!crypto_pk_read_private_key_from_string(sign_skey_1,
-                                                       AUTHORITY_SIGNKEY_1));
+                                                       AUTHORITY_SIGNKEY_1,-1));
    test_assert(!crypto_pk_read_private_key_from_string(sign_skey_2,
-                                                       AUTHORITY_SIGNKEY_2));
+                                                       AUTHORITY_SIGNKEY_2,-1));
    test_assert(!crypto_pk_read_private_key_from_string(sign_skey_3,
-                                                       AUTHORITY_SIGNKEY_3));
+                                                       AUTHORITY_SIGNKEY_3,-1));
  
    test_assert(!crypto_pk_cmp_keys(sign_skey_1, cert1->signing_key));
    test_assert(!crypto_pk_cmp_keys(sign_skey_2, cert2->signing_key));