From 03efdd03323660138b537d9ae3fef591f0c51986 Mon Sep 17 00:00:00 2001 From: kanoi Date: Tue, 18 Nov 2014 12:29:10 +1100 Subject: [PATCH] ckdb - disallow web usernames like BTC addresses --- src/ckdb.c | 8 ++++++-- src/ckdb.h | 11 ++++++++++- src/ckdb_cmd.c | 37 +++++++++++++++++++++++++------------ src/ckdb_data.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 15 deletions(-) diff --git a/src/ckdb.c b/src/ckdb.c index 3554a11e..c43ae064 100644 --- a/src/ckdb.c +++ b/src/ckdb.c @@ -174,8 +174,12 @@ const char *mailpatt = "^[A-Za-z0-9_-][A-Za-z0-9_\\.-]*@[A-Za-z0-9][A-Za-z0-9\\. const char *idpatt = "^[_A-Za-z][_A-Za-z0-9]*$"; const char *intpatt = "^[0-9][0-9]*$"; const char *hashpatt = "^[A-Fa-f0-9]*$"; -// TODO: bitcoind will check it properly -const char *addrpatt = "^[A-Za-z0-9]*$"; +/* BTC addresses start with '1' (one) or '3' (three), + * exclude: capital 'I' (eye), capital 'O' (oh), + * lowercase 'l' (elle) and '0' (zero) + * and with a simple test must be ADDR_MIN_LEN to ADDR_MAX_LEN (ckdb.h) + * bitcoind is used to fully validate them when required */ +const char *addrpatt = "^[13][A-HJ-NP-Za-km-z1-9]*$"; // So the records below have the same 'name' as the klist const char Transfer[] = "Transfer"; diff --git a/src/ckdb.h b/src/ckdb.h index dd245ef3..f9a216c0 100644 --- a/src/ckdb.h +++ b/src/ckdb.h @@ -52,7 +52,7 @@ #define DB_VLOCK "1" #define DB_VERSION "0.9.4" -#define CKDB_VERSION DB_VERSION"-0.631" +#define CKDB_VERSION DB_VERSION"-0.640" #define WHERE_FFL " - from %s %s() line %d" #define WHERE_FFL_HERE __FILE__, __func__, __LINE__ @@ -80,6 +80,14 @@ extern const char *intpatt; extern const char *hashpatt; extern const char *addrpatt; +/* If a trimmed username is like an address but this many or more characters, + * disallow it */ +#define ADDR_USER_CHECK 16 + +// BTC address size +#define ADDR_MIN_LEN 26 +#define ADDR_MAX_LEN 34 + typedef struct loadstatus { tv_t oldest_sharesummary_firstshare_n; tv_t newest_sharesummary_firstshare_a; @@ -1334,6 +1342,7 @@ extern PGconn *dbconnect(); extern char *safe_text(char *txt); extern void username_trim(USERS *users); +extern bool like_address(char *username); extern void _txt_to_data(enum data_type typ, char *nam, char *fld, void *data, size_t siz, WHERE_FFL_ARGS); diff --git a/src/ckdb_cmd.c b/src/ckdb_cmd.c index c7c580c8..e3443858 100644 --- a/src/ckdb_cmd.c +++ b/src/ckdb_cmd.c @@ -15,7 +15,7 @@ static char *cmd_adduser(PGconn *conn, char *cmd, char *id, tv_t *now, char *by, { char reply[1024] = ""; size_t siz = sizeof(reply); - K_ITEM *i_username, *i_emailaddress, *i_passwordhash, *u_item; + K_ITEM *i_username, *i_emailaddress, *i_passwordhash, *u_item = NULL; LOGDEBUG("%s(): cmd '%s'", __func__, cmd); @@ -23,18 +23,30 @@ static char *cmd_adduser(PGconn *conn, char *cmd, char *id, tv_t *now, char *by, if (!i_username) return strdup(reply); - i_emailaddress = require_name(trf_root, "emailaddress", 7, (char *)mailpatt, reply, siz); - if (!i_emailaddress) - return strdup(reply); + /* If a username added from the web site looks like an address + * then disallow it - a false positive is not an issue + * Allowing it will create a security issue - someone could create + * an account with someone else's, as yet unused, payout address + * and redirect the payout to another payout address. + * ... and the person who owns the payout address can't check that + * in advance, they'll just find out with their first payout not + * arriving at their payout address */ + if (!like_address(transfer_data(i_username))) { + i_emailaddress = require_name(trf_root, "emailaddress", 7, + (char *)mailpatt, reply, siz); + if (!i_emailaddress) + return strdup(reply); - i_passwordhash = require_name(trf_root, "passwordhash", 64, (char *)hashpatt, reply, siz); - if (!i_passwordhash) - return strdup(reply); + i_passwordhash = require_name(trf_root, "passwordhash", 64, + (char *)hashpatt, reply, siz); + if (!i_passwordhash) + return strdup(reply); - u_item = users_add(conn, transfer_data(i_username), - transfer_data(i_emailaddress), - transfer_data(i_passwordhash), - by, code, inet, now, trf_root); + u_item = users_add(conn, transfer_data(i_username), + transfer_data(i_emailaddress), + transfer_data(i_passwordhash), + by, code, inet, now, trf_root); + } if (!u_item) { LOGERR("%s() %s.failed.DBE", __func__, id); @@ -223,7 +235,8 @@ static char *cmd_userset(PGconn *conn, char *cmd, char *id, email = NULL; } i_address = optional_name(trf_root, "address", - 27, (char *)addrpatt, + ADDR_MIN_LEN, + (char *)addrpatt, reply, siz); if (i_address) address = transfer_data(i_address); diff --git a/src/ckdb_data.c b/src/ckdb_data.c index e712f520..b799648d 100644 --- a/src/ckdb_data.c +++ b/src/ckdb_data.c @@ -67,6 +67,55 @@ void username_trim(USERS *users) } } +/* Is the trimmed username like an address? + * False positive is OK (i.e. 'like') + * Before checking, it is trimmed to avoid web display confusion + * Length check is done before trimming - this may give a false + * positive on any username with lots of trim characters ... which is OK */ +bool like_address(char *username) +{ + char *tmp, *front, *trail; + size_t len; + regex_t re; + int ret; + + len = strlen(username); + if (len < ADDR_USER_CHECK) + return false; + + tmp = strdup(username); + front = tmp; + while (*front && TRIM_IGNORE(*front)) + front++; + + trail = front + strlen(front) - 1; + while (trail >= front) { + if (TRIM_IGNORE(*trail)) + *(trail--) = '\0'; + else + break; + } + + if (regcomp(&re, addrpatt, REG_NOSUB) != 0) { + LOGEMERG("%s(): failed to compile addrpatt '%s'", + __func__, addrpatt); + free(tmp); + // This will disable adding any new usernames ... + return true; + } + + ret = regexec(&re, front, (size_t)0, NULL, 0); + regfree(&re); + + if (ret == 0) { + free(tmp); + return true; + } + + free(tmp); + return false; +} + void _txt_to_data(enum data_type typ, char *nam, char *fld, void *data, size_t siz, WHERE_FFL_ARGS) { char *tmp;