From bac5831b355f916e0696b7bbcccfc51c057b729a Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 5 Jun 2014 17:10:43 +0000 Subject: [PATCH 1/5] Bugfix: initiate_stratum: Ensure extranonce2 size is not negative (which could lead to exploits later as too little memory gets allocated) Thanks to Mick Ayzenberg for finding this! --- util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util.c b/util.c index d1846437..16aa98a4 100644 --- a/util.c +++ b/util.c @@ -2505,7 +2505,8 @@ resend: goto out; } n2size = json_integer_value(json_array_get(res_val, 2)); - if (!n2size) { + if (n2size < 1) + { applog(LOG_INFO, "Failed to get n2size in initiate_stratum"); free(sessionid); free(nonce1); From b65574bef233474e915fdf18614aa211e31cc6c2 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 5 Jun 2014 17:05:12 +0000 Subject: [PATCH 2/5] Stratum: extract_sockaddr: Truncate overlong addresses rather than stack overflow Thanks to Mick Ayzenberg for finding this! --- util.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/util.c b/util.c index 16aa98a4..77f20431 100644 --- a/util.c +++ b/util.c @@ -1219,6 +1219,13 @@ bool extract_sockaddr(char *url, char **sockaddr_url, char **sockaddr_port) if (url_len < 1) return false; + + if (url_len >= sizeof(url_address)) + { + applog(LOG_WARNING, "%s: Truncating overflowed address '%.*s'", + __func__, url_len, url_begin); + url_len = sizeof(url_address) - 1; + } sprintf(url_address, "%.*s", url_len, url_begin); From 78cc408369bdbbd440196c93574098d1482efbce Mon Sep 17 00:00:00 2001 From: Noel Maersk Date: Thu, 5 Jun 2014 21:45:01 +0300 Subject: [PATCH 3/5] stratum: parse_reconnect(): treat pool-sent URL as untrusted. Thanks to Mick Ayzenberg for reminding that this existed and highlighting the offender. Also to Luke-jr for actually fixing this in bfgminer. :D --- util.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/util.c b/util.c index 77f20431..94154518 100644 --- a/util.c +++ b/util.c @@ -1718,15 +1718,14 @@ static void __suspend_stratum(struct pool *pool) static bool parse_reconnect(struct pool *pool, json_t *val) { - char *sockaddr_url, *stratum_port, *tmp; - char *url, *port, address[256]; - if (opt_disable_client_reconnect) { - applog(LOG_WARNING, "Stratum client.reconnect forbidden, aborting."); + applog(LOG_WARNING, "Stratum client.reconnect received but is disabled, not reconnecting."); return false; } - memset(address, 0, 255); + char *url, *port, address[256]; + char *sockaddr_url, *stratum_port, *tmp; /* Tempvars. */ + url = (char *)json_string_value(json_array_get(val, 0)); if (!url) url = pool->sockaddr_url; @@ -1735,8 +1734,7 @@ static bool parse_reconnect(struct pool *pool, json_t *val) if (!port) port = pool->stratum_port; - sprintf(address, "%s:%s", url, port); - + snprintf(address, sizeof(address), "%s:%s", url, port); if (!extract_sockaddr(address, &sockaddr_url, &stratum_port)) return false; From 4f387321cea8907130ab3dc6d52e6c9bf1f7f9af Mon Sep 17 00:00:00 2001 From: Noel Maersk Date: Thu, 5 Jun 2014 22:02:14 +0300 Subject: [PATCH 4/5] misc: update AUTHORS.md, add CR to sgminer.c, minor style. --- AUTHORS.md | 3 +-- sgminer.c | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/AUTHORS.md b/AUTHORS.md index 77e559ab..20d634e5 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -3,7 +3,7 @@ ## Core * Multiple algorithms and switching: Jan Berdajs 15bULC8snaKAMeFb3xBmmhbWj1xyTmBUfm -* Scrypt-only refactor: Noel Maersk LfxRFgXzA13TSTxgFGUFEtumv5ShGzAmLw +* Historical scrypt-only refactor: Noel Maersk 12jF1VExtmmMu8D36vo4Y4CYqLK5yCtLC4 * Core: Martin Danielsen 1DNBcSEENBwDKrcTyTW61ezWhzsPy5imkn * Core: Con Kolivas 15qSxP1SQcUX3o4nhkfdbgyoWEFMomJ4rZ * Core: Luke Dashjr 1QATWksNFGeUJCWBrN4g6hGM178Lovm7Wh @@ -33,7 +33,6 @@ updated by many others. ## Testing, bug fixes, improvements -* Hot-switching OpenCL kernel (N-Factor): Jan Berdajs * Michael Fiano * Gabriel Devenyi * Benjamin Herrenschmidt diff --git a/sgminer.c b/sgminer.c index b2687cdb..020fbd36 100644 --- a/sgminer.c +++ b/sgminer.c @@ -1,4 +1,5 @@ /* + * Copyright 2013-2014 sgminer developers (see AUTHORS.md) * Copyright 2011-2013 Con Kolivas * Copyright 2011-2012 Luke Dashjr * Copyright 2010 Jeff Garzik @@ -756,8 +757,7 @@ static void setup_url(struct pool *pool, char *arg) return; opt_set_charp(arg, &pool->rpc_url); - if (strncmp(arg, "http://", 7) && - strncmp(arg, "https://", 8)) { + if (strncmp(arg, "http://", 7) && strncmp(arg, "https://", 8)) { char *httpinput; httpinput = (char *)malloc(255); From 910c36089940e81fb85c65b8e63dcd2fac71470c Mon Sep 17 00:00:00 2001 From: Noel Maersk Date: Thu, 5 Jun 2014 23:02:02 +0300 Subject: [PATCH 5/5] stratum: parse_notify(): Don't die on malformed bbversion/prev_hash/nbit/ntime. Might have introduced a memory leak, don't have time to check. :( Should the other hex2bin()'s be checked? Thanks to Mick Ayzenberg for finding this. --- util.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/util.c b/util.c index 94154518..b3d2f0b3 100644 --- a/util.c +++ b/util.c @@ -1600,17 +1600,23 @@ static bool parse_notify(struct pool *pool, json_t *val) pool->swork.nbit, "00000000", /* nonce */ workpadding); - if (unlikely(!hex2bin(pool->header_bin, header, 128))) - quit(1, "Failed to convert header to header_bin in parse_notify"); + if (unlikely(!hex2bin(pool->header_bin, header, 128))) { + applog(LOG_WARNING, "%s: Failed to convert header to header_bin, got %s", __func__, header); + pool_failed(pool); + // TODO: memory leaks? goto out, clean up there? + return false; + } cb1 = (unsigned char *)calloc(cb1_len, 1); if (unlikely(!cb1)) quithere(1, "Failed to calloc cb1 in parse_notify"); hex2bin(cb1, coinbase1, cb1_len); + cb2 = (unsigned char *)calloc(cb2_len, 1); if (unlikely(!cb2)) quithere(1, "Failed to calloc cb2 in parse_notify"); hex2bin(cb2, coinbase2, cb2_len); + free(pool->coinbase); align_len(&alloc_len); pool->coinbase = (unsigned char *)calloc(alloc_len, 1); @@ -1618,6 +1624,7 @@ static bool parse_notify(struct pool *pool, json_t *val) quit(1, "Failed to calloc pool coinbase in parse_notify"); memcpy(pool->coinbase, cb1, cb1_len); memcpy(pool->coinbase + cb1_len, pool->nonce1bin, pool->n1_len); + // NOTE: gap for nonce2, filled at work generation time memcpy(pool->coinbase + cb1_len + pool->n1_len + pool->n2size, cb2, cb2_len); cg_wunlock(&pool->data_lock);