commit 6ba1db07b516763ef62df933fb46d88470d6a805 Author: Zack Weinberg zackw@panix.com Date: Mon Jul 18 12:23:20 2011 -0700
Introduce allocate-memory-or-crash helpers and use them throughout the code base. --- src/crypt.c | 26 ++++++--------- src/crypt.h | 11 +++--- src/main.c | 34 ++++---------------- src/network.c | 25 ++++++--------- src/protocols/dummy.c | 6 +-- src/protocols/obfs2.c | 34 +++++--------------- src/socks.c | 28 ++++++---------- src/socks.h | 2 +- src/test/unittest_obfs2.c | 3 +- src/test/unittest_socks.c | 5 --- src/util.c | 76 +++++++++++++++++++++++++++++++++++++++++--- src/util.h | 13 ++++++++ 12 files changed, 138 insertions(+), 125 deletions(-)
diff --git a/src/crypt.c b/src/crypt.c index 6be9726..8d1d607 100644 --- a/src/crypt.c +++ b/src/crypt.c @@ -4,9 +4,11 @@
#define CRYPT_PRIVATE #include "crypt.h" +#include "util.h"
#include <assert.h> #include <fcntl.h> +#include <limits.h> #include <stdlib.h> #include <string.h> #include <unistd.h> @@ -74,14 +76,12 @@ struct digest_t { };
/** - Returns a new SHA256 digest container, or NULL on failure. + Returns a new SHA256 digest container. */ digest_t * digest_new(void) { - digest_t *d = malloc(sizeof(digest_t)); - if (!d) - return NULL; + digest_t *d = xmalloc(sizeof(digest_t)); SHA256_Init(&d->ctx); return d; } @@ -89,7 +89,7 @@ digest_new(void) /** Updates the contents of the SHA256 container 'd' with the first 'len' bytes of 'buf'. -*/ +*/ void digest_update(digest_t *d, const uchar *buf, size_t len) { @@ -118,9 +118,7 @@ struct digest_t { digest_t * digest_new(void) { - digest_t *d = malloc(sizeof(digest_t)); - if (!d) - return NULL; + digest_t *d = xmalloc(sizeof(digest_t)); sha256_init(&d->ctx); return d; } @@ -156,19 +154,15 @@ digest_free(digest_t *d)
/** Initializes the AES cipher with 'key'. -*/ +*/ crypt_t * crypt_new(const uchar *key, size_t keylen) { crypt_t *k; - if (keylen < AES_BLOCK_SIZE) - return NULL; - - k = calloc(1, sizeof(crypt_t)); - if (k == NULL) - return NULL;
- AES_set_encrypt_key(key, 128, &k->key); + assert(keylen == AES_BLOCK_SIZE); + k = xzalloc(sizeof(crypt_t)); + AES_set_encrypt_key(key, AES_BLOCK_SIZE * CHAR_BIT, &k->key);
return k; } diff --git a/src/crypt.h b/src/crypt.h index d87b1de..3f1e4df 100644 --- a/src/crypt.h +++ b/src/crypt.h @@ -21,7 +21,7 @@ int initialize_crypto(void); /** Clean up global crypto state */ void cleanup_crypto(void);
-/** Return a newly allocated digest state, or NULL on failure. */ +/** Return a newly allocated digest state; cannot fail. */ digest_t *digest_new(void); /** Add n bytes from b to the digest state. */ void digest_update(digest_t *, const uchar *b, size_t n); @@ -31,10 +31,11 @@ size_t digest_getdigest(digest_t *, uchar *b, size_t n); /** Clear and free a digest state */ void digest_free(digest_t *);
-/** Return a new stream cipher state taking key and IV from the data provided. - * The data length must be exactly 32 */ -crypt_t *crypt_new(const uchar *, size_t); -void crypt_set_iv(crypt_t *key, const uchar *iv, size_t ivlen); +/** Return a new stream cipher state using 'key' as the symmetric key. + * The data length must be exactly 16 bytes. Cannot fail. */ +crypt_t *crypt_new(const uchar *key, size_t); +/* Set the IV of a stream-cipher state. Cannot fail. */ +void crypt_set_iv(crypt_t *, const uchar *iv, size_t ivlen);
/** Encrypt n bytes of data in the buffer b, in place. */ void stream_crypt(crypt_t *, uchar *b, size_t n); diff --git a/src/main.c b/src/main.c index f41d502..21c4feb 100644 --- a/src/main.c +++ b/src/main.c @@ -181,13 +181,6 @@ handle_obfsproxy_args(const char **argv) return i; }
-static void -die_oom(void) -{ - log_warn("Memory allocation failed: %s",strerror(errno)); - exit(1); -} - int main(int argc, const char **argv) { @@ -214,7 +207,6 @@ main(int argc, const char **argv) int start; int end; int n_options; - void *realloc_temp; int i;
/* The number of protocols. */ @@ -222,7 +214,7 @@ main(int argc, const char **argv) /* An array which holds the position in argv of the command line options for each protocol. */ unsigned int *protocols=NULL; - /* keeps track of allocated space for the protocols array */ + /* keeps track of allocated space for the protocols array */ unsigned int n_alloc;
if (argc < 2) { @@ -235,9 +227,7 @@ main(int argc, const char **argv) /** Handle optional obfsproxy arguments. */ start_of_protocols = handle_obfsproxy_args(&argv[1]);
- protocols = calloc(sizeof(int), (n_protocols+1)); - if (!protocols) - die_oom(); + protocols = xzalloc((n_protocols + 1) * sizeof(int)); n_alloc = n_protocols+1;
/* Populate protocols and calculate n_protocols. */ @@ -249,10 +239,7 @@ main(int argc, const char **argv) /* Do we need to expand the protocols array? */ if (n_alloc <= n_protocols) { n_alloc *= 2; - realloc_temp = realloc(protocols, sizeof(int)*(n_alloc)); - if (!realloc_temp) - die_oom(); - protocols = realloc_temp; + protocols = xrealloc(protocols, sizeof(int)*(n_alloc)); } } } @@ -271,13 +258,9 @@ main(int argc, const char **argv) that point to arrays carrying the options of the protocols. Finally, we allocate enough space on the n_options_array so that we can put the number of options there. - */ - protocol_options = calloc(sizeof(char**), n_protocols); - if (!protocol_options) - die_oom(); - n_options_array = calloc(sizeof(int), n_protocols); - if (!n_options_array) - die_oom(); + */ + protocol_options = xzalloc(n_protocols * sizeof(char**)); + n_options_array = xzalloc(n_protocols * sizeof(int));
/* Iterate through protocols. */ for (i=0;i<n_protocols;i++) { @@ -304,10 +287,7 @@ main(int argc, const char **argv)
/* Allocate space for the array carrying the options of this protocol. */ - protocol_options[actual_protocols-1] = - calloc(sizeof(char*), (n_options)); - if (!protocol_options[actual_protocols-1]) - die_oom(); + protocol_options[actual_protocols-1] = xzalloc(n_options * sizeof(char*));
/* Write the number of options to the correct place in n_options_array[]. */ n_options_array[actual_protocols-1] = n_options; diff --git a/src/network.c b/src/network.c index 365c61a..fb770c9 100644 --- a/src/network.c +++ b/src/network.c @@ -99,10 +99,15 @@ close_all_connections(void) assert(!n_connections); } /** - This function spawns a listener according to the 'proto_params'. + This function spawns a listener configured according to the + provided 'protocol_params_t' object'. Returns the listener on + success, NULL on fail.
- Returns the listener on success, NULL on fail. + If it succeeds, the new listener object takes ownership of the + protocol_params_t object provided; if it fails, the protocol_params_t + object is deallocated. */ + listener_t * listener_new(struct event_base *base, protocol_params_t *proto_params) @@ -110,14 +115,8 @@ listener_new(struct event_base *base, const unsigned flags = LEV_OPT_CLOSE_ON_FREE|LEV_OPT_CLOSE_ON_EXEC|LEV_OPT_REUSEABLE;
- listener_t *lsn = calloc(1, sizeof(listener_t)); - if (!lsn) { - if (proto_params) - free(proto_params); - return NULL; - } + listener_t *lsn = xzalloc(sizeof(listener_t));
- /** If we don't have a connection dll, create one now. */ lsn->proto_params = proto_params;
lsn->listener = evconnlistener_new_bind(base, simple_listener_cb, lsn, @@ -132,6 +131,7 @@ listener_new(struct event_base *base, return NULL; }
+ /** If we don't have a connection dll, create one now. */ dll_append(&listener_list, &lsn->dll_node);
return lsn; @@ -189,15 +189,12 @@ simple_listener_cb(struct evconnlistener *evcl, { listener_t *lsn = arg; struct event_base *base; - conn_t *conn = calloc(1, sizeof(conn_t)); + conn_t *conn = xzalloc(sizeof(conn_t));
n_connections++; /* If we call conn_free() later on error, it will decrement * n_connections. Therefore, we had better increment it at * the start. */
- if (!conn) - goto err; - log_debug("Got a connection attempt.");
conn->mode = lsn->proto_params->mode; @@ -211,8 +208,6 @@ simple_listener_cb(struct evconnlistener *evcl, if (conn->mode == LSN_SOCKS_CLIENT) { /* Construct SOCKS state. */ conn->socks_state = socks_state_new(); - if (!conn->socks_state) - goto err; }
/* New bufferevent to wrap socket we received. */ diff --git a/src/protocols/dummy.c b/src/protocols/dummy.c index f121725..d82818d 100644 --- a/src/protocols/dummy.c +++ b/src/protocols/dummy.c @@ -28,9 +28,7 @@ static struct protocol_params_t * dummy_init(int n_options, const char *const *options) { struct protocol_params_t *params - = calloc(1, sizeof(struct protocol_params_t)); - if (!params) - return NULL; + = xzalloc(sizeof(struct protocol_params_t));
if (parse_and_set_options(n_options, options, params) < 0) { free(params); @@ -101,7 +99,7 @@ static struct protocol_t * dummy_create(struct protocol_params_t *params) { /* Dummy needs no per-connection protocol-specific state. */ - struct protocol_t *proto = calloc(1, sizeof(struct protocol_t)); + struct protocol_t *proto = xzalloc(sizeof(struct protocol_t)); proto->vtable = &dummy_vtable; return proto; } diff --git a/src/protocols/obfs2.c b/src/protocols/obfs2.c index 41a2614..c42d1c4 100644 --- a/src/protocols/obfs2.c +++ b/src/protocols/obfs2.c @@ -36,9 +36,7 @@ static struct protocol_params_t * obfs2_init(int n_options, const char *const *options) { struct protocol_params_t *params - = calloc(1, sizeof(struct protocol_params_t)); - if (!params) - return NULL; + = xzalloc(sizeof(struct protocol_params_t));
if (parse_and_set_options(n_options, options, params) < 0) { usage(); @@ -85,8 +83,9 @@ parse_and_set_options(int n_options, const char *const *options, if (got_ss) return -1; /* this is freed in proto_params_free() */ - params->shared_secret = strdup(*options+16); params->shared_secret_len = strlen(*options+16); + params->shared_secret = xmemdup(*options+16, + params->shared_secret_len + 1); got_ss=1; } else { log_warn("obfs2: Unknown argument."); @@ -163,7 +162,7 @@ seed_nonzero(const uchar *seed)
/** Derive and return key of type 'keytype' from the seeds currently set in - 'state'. Returns NULL on failure. + 'state'. */ static crypt_t * derive_key(void *s, const char *keytype) @@ -202,7 +201,7 @@ derive_key(void *s, const char *keytype)
/** Derive and return padding key of type 'keytype' from the seeds - currently set in state 's'. Returns NULL on failure. + currently set in state 's'. */ static crypt_t * derive_padding_key(void *s, const uchar *seed, @@ -250,12 +249,10 @@ derive_padding_key(void *s, const uchar *seed, static struct protocol_t * obfs2_create(protocol_params_t *params) { - obfs2_protocol_t *proto = calloc(1, sizeof(obfs2_protocol_t)); + obfs2_protocol_t *proto = xzalloc(sizeof(obfs2_protocol_t)); uchar *seed; const char *send_pad_type;
- if (!proto) - return NULL; proto->state = ST_WAIT_FOR_KEY; proto->we_are_initiator = params->is_initiator; if (proto->we_are_initiator) { @@ -275,10 +272,6 @@ obfs2_create(protocol_params_t *params) if (params->shared_secret) { /* ASN we must say in spec that we hash command line shared secret. */ digest_t *c = digest_new(); - if (!c) { - free(proto); - return NULL; - } digest_update(c, (uchar*)params->shared_secret, params->shared_secret_len); digest_getdigest(c, proto->secret_seed, SHARED_SECRET_LENGTH); digest_free(c); @@ -286,11 +279,6 @@ obfs2_create(protocol_params_t *params)
/* Derive the key for what we're sending */ proto->send_padding_crypto = derive_padding_key(proto, seed, send_pad_type); - if (proto->send_padding_crypto == NULL) { - free(proto); - return NULL; - } - proto->super.vtable = &obfs2_vtable; return &proto->super; } @@ -420,7 +408,7 @@ obfs2_send(struct protocol_t *s, Helper: called after reciving our partner's setup message. Initializes all keys. Returns 0 on success, -1 on failure. */ -static int +static void init_crypto(void *s) { obfs2_protocol_t *state = s; @@ -447,11 +435,6 @@ init_crypto(void *s) state->recv_crypto = derive_key(state, recv_keytype); state->recv_padding_crypto = derive_padding_key(state, recv_seed, recv_pad_keytype); - - if (state->send_crypto && state->recv_crypto && state->recv_padding_crypto) - return 0; - else - return -1; }
/* Called when we receive data in an evbuffer 'source': deobfuscates that data @@ -491,8 +474,7 @@ obfs2_recv(struct protocol_t *s, struct evbuffer *source, memcpy(other_seed, buf, OBFUSCATE_SEED_LENGTH);
/* Now we can set up all the keys from the seed */ - if (init_crypto(state) < 0) - return RECV_BAD; + init_crypto(state);
/* Decrypt the next 8 bytes */ stream_crypt(state->recv_padding_crypto, buf+OBFUSCATE_SEED_LENGTH, 8); diff --git a/src/socks.c b/src/socks.c index e89812f..b67455f 100644 --- a/src/socks.c +++ b/src/socks.c @@ -43,24 +43,19 @@ typedef unsigned char uchar;
/** - Creates a new SOCKS state. - - Returns a 'socks_state_t' on success, NULL on fail. + Creates a new 'socks_state_t' object. */ socks_state_t * socks_state_new(void) { - socks_state_t *state = calloc(1, sizeof(socks_state_t)); - if (!state) - return NULL; + socks_state_t *state = xzalloc(sizeof(socks_state_t)); state->state = ST_WAITING; - return state; }
/** Deallocates memory of socks_state_t 's'. -*/ +*/ void socks_state_free(socks_state_t *s) { @@ -318,8 +313,8 @@ socks5_handle_negotiation(struct evbuffer *source, struct evbuffer *dest, socks_state_t *state) { unsigned int found_noauth, i; - uchar nmethods; + uchar methods[0xFF];
evbuffer_copyout(source, &nmethods, 1);
@@ -329,25 +324,22 @@ socks5_handle_negotiation(struct evbuffer *source,
evbuffer_drain(source, 1);
- uchar *p; - /* XXX user controlled malloc(). range should be: 0x00-0xff */ - p = malloc(nmethods); - if (!p) { - log_warn("malloc failed!"); + /* this should be impossible, but we check it anyway for great defensiveness */ + if (nmethods > 0xFF) { + log_warn("too many methods!"); return SOCKS_BROKEN; } - if (evbuffer_remove(source, p, nmethods) < 0) + + if (evbuffer_remove(source, methods, nmethods) < 0) assert(0);
for (found_noauth=0, i=0; i<nmethods ; i++) { - if (p[i] == SOCKS5_METHOD_NOAUTH) { + if (methods[i] == SOCKS5_METHOD_NOAUTH) { found_noauth = 1; break; } }
- free(p); - return socks5_do_negotiation(dest,found_noauth); }
diff --git a/src/socks.h b/src/socks.h index e2c20b5..db10cb5 100644 --- a/src/socks.h +++ b/src/socks.h @@ -30,7 +30,7 @@ enum socks_ret { enum socks_ret handle_socks(struct evbuffer *source, struct evbuffer *dest, socks_state_t *socks_state); -socks_state_t *socks_state_new(void); +socks_state_t *socks_state_new(void); /* cannot fail */ void socks_state_free(socks_state_t *s);
enum socks_status_t socks_state_get_status(const socks_state_t *state); diff --git a/src/test/unittest_obfs2.c b/src/test/unittest_obfs2.c index 01ce501..961bfd2 100644 --- a/src/test/unittest_obfs2.c +++ b/src/test/unittest_obfs2.c @@ -125,8 +125,7 @@ static const char *const options_server[] = static void * setup_obfs2_state(const struct testcase_t *unused) { - struct test_obfs2_state *s = calloc(1, sizeof(struct test_obfs2_state)); - tt_assert(s); + struct test_obfs2_state *s = xzalloc(sizeof(struct test_obfs2_state));
s->proto_params_client = proto_params_init(ALEN(options_client), options_client); diff --git a/src/test/unittest_socks.c b/src/test/unittest_socks.c index 001c266..4d95666 100644 --- a/src/test/unittest_socks.c +++ b/src/test/unittest_socks.c @@ -31,7 +31,6 @@ test_socks_socks5_send_negotiation(void *data)
socks_state_t *state; state = socks_state_new(); - tt_assert(state);
/* First test: Only one method: NOAUTH. @@ -143,7 +142,6 @@ test_socks_socks5_request(void *data)
socks_state_t *state; state = socks_state_new(); - tt_assert(state);
const uint32_t addr_ipv4 = htonl(0x7f000001); /* 127.0.0.1 */ const uint8_t addr_ipv6[16] = {0,13,0,1,0,5,0,14,0,10,0,5,0,14,0,0}; /* d:1:5:e:a:5:e:0 */ @@ -308,7 +306,6 @@ test_socks_socks5_request_reply(void *data)
socks_state_t *state; state = socks_state_new(); - tt_assert(state);
state->parsereq.af = AF_INET; strcpy(state->parsereq.addr, "127.0.0.1"); @@ -415,7 +412,6 @@ test_socks_socks4_request(void *data)
socks_state_t *state; state = socks_state_new(); - tt_assert(state);
/* First test: Correct SOCKS4 req packet with nothing in the optional field. */ @@ -552,7 +548,6 @@ test_socks_socks4_request_reply(void *data)
socks_state_t *state; state = socks_state_new(); - tt_assert(state);
state->parsereq.af = AF_INET; strcpy(state->parsereq.addr, "127.0.0.1"); diff --git a/src/util.c b/src/util.c index cab4bf1..6a0e4a0 100644 --- a/src/util.c +++ b/src/util.c @@ -5,6 +5,7 @@ #include "util.h"
#include <assert.h> +#include <errno.h> #include <fcntl.h> #include <limits.h> #include <stdio.h> @@ -22,6 +23,73 @@ /** Any size_t larger than this amount is likely to be an underflow. */ #define SIZE_T_CEILING (SIZE_MAX/2 - 16)
+/**************************** Memory Allocation ******************************/ + +static void __attribute__((noreturn)) +die_oom(void) +{ + log_warn("Memory allocation failed: %s",strerror(errno)); + exit(1); +} + +void * +xmalloc(size_t size) +{ + void *result; + + assert(size < SIZE_T_CEILING); + + /* Some malloc() implementations return NULL when the input argument + is zero. We don't bother detecting whether the implementation we're + being compiled for does that, because it should hardly ever come up, + and avoiding it unconditionally does no harm. */ + if (size == 0) + size = 1; + + result = malloc(size); + if (result == NULL) + die_oom(); + + return result; +} + +void * +xrealloc(void *ptr, size_t size) +{ + void *result; + assert (size < SIZE_T_CEILING); + if (size == 0) + size = 1; + + result = realloc(ptr, size); + if (result == NULL) + die_oom(); + + return result; +} + +void * +xzalloc(size_t size) +{ + void *result = xmalloc(size); + memset(result, 0, size); + return result; +} + +void * +xmemdup(const void *ptr, size_t size) +{ + void *copy = xmalloc(size); + memcpy(copy, ptr, size); + return copy; +} + +char * +xstrdup(const char *s) +{ + return xmemdup(s, strlen(s) + 1); +} + /************************ Obfsproxy Network Routines *************************/
/** @@ -45,10 +113,8 @@ resolve_address_port(const char *address, struct evutil_addrinfo *ai = NULL; struct evutil_addrinfo ai_hints; int result = -1, ai_res; - char *a = strdup(address), *cp; + char *a = xstrdup(address), *cp; const char *portstr; - if (!a) - return -1;
if ((cp = strchr(a, ':'))) { portstr = cp+1; @@ -78,10 +144,8 @@ resolve_address_port(const char *address, log_warn("No result for address %s", address); goto done; } - struct sockaddr *addr = malloc(ai->ai_addrlen); - memcpy(addr, ai->ai_addr, ai->ai_addrlen); - *addr_out = addr; *addrlen_out = ai->ai_addrlen; + *addr_out = xmemdup(ai->ai_addr, ai->ai_addrlen); result = 0;
done: diff --git a/src/util.h b/src/util.h index b526dbf..d909b53 100644 --- a/src/util.h +++ b/src/util.h @@ -16,6 +16,19 @@ struct sockaddr; struct event_base; struct evdns_base;
+/***** Memory allocation. *****/ + +/* Because this isn't Tor and functions named "tor_whatever" would be + confusing, I am instead following the GNU convention of naming + allocate-memory-or-crash functions "xwhatever". Also, at this time + I do not see a need for a free() wrapper. */ + +void *xmalloc(size_t size) __attribute__((malloc)); /* does not clear memory */ +void *xzalloc(size_t size) __attribute__((malloc)); /* clears memory */ +void *xrealloc(void *ptr, size_t size); +void *xmemdup(const void *ptr, size_t size) __attribute__((malloc)); +char *xstrdup(const char *s) __attribute__((malloc)); + /***** Network functions stuff. *****/
int resolve_address_port(const char *address,