From 91e898971b8ec935b2d25e7cd0d26eb0c6a8d8a7 Mon Sep 17 00:00:00 2001 From: divverent Date: Wed, 26 Apr 2017 02:33:43 +0000 Subject: [PATCH] rcon server: add an explicit check against empty password, instead of assuming the calling code is correct. This logic had a bug in the past that had been introduced by r8886 and accidentally fixed when introducing multi-account support in r9420. The bug was that rcon_password being empty led to the empty password being accepted in some cases, as there was never any explicit logic to handle that in rcon - it was more "accidentally" rejected by a packet formatting check which does not apply to srcon as the empty password wouldn't be empty over the wire. In other words: DP builds from [r8886, r9419] had a serious vulnerability and the only workaround is to always have rcon_password set. The check added here will complain and reject in case the calling code for some reason no longer prevents empty passwords from being accepted. Also adding comments to the new logic explaining how it prevents the empty password from being accepted. git-svn-id: svn://svn.icculus.org/twilight/trunk/darkplaces@12334 d7cf8633-e32d-0410-b094-e92efae38249 --- netconn.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/netconn.c b/netconn.c index 97f07bac..342d78b2 100755 --- a/netconn.c +++ b/netconn.c @@ -2739,6 +2739,11 @@ static qboolean hmac_mdfour_time_matching(lhnetaddress_t *peeraddress, const cha char mdfourbuf[16]; long t1, t2; + if (!password[0]) { + Con_Print("^4LOGIC ERROR: RCon_Authenticate should never call the comparator with an empty password. Please report.\n"); + return false; + } + t1 = (long) time(NULL); t2 = strtol(s, NULL, 0); if(abs(t1 - t2) > rcon_secure_maxdiff.integer) @@ -2755,6 +2760,11 @@ static qboolean hmac_mdfour_challenge_matching(lhnetaddress_t *peeraddress, cons char mdfourbuf[16]; int i; + if (!password[0]) { + Con_Print("^4LOGIC ERROR: RCon_Authenticate should never call the comparator with an empty password. Please report.\n"); + return false; + } + if(slen < (int)(sizeof(challenges[0].string)) - 1) return false; @@ -2781,6 +2791,11 @@ static qboolean hmac_mdfour_challenge_matching(lhnetaddress_t *peeraddress, cons static qboolean plaintext_matching(lhnetaddress_t *peeraddress, const char *password, const char *hash, const char *s, int slen) { + if (!password[0]) { + Con_Print("^4LOGIC ERROR: RCon_Authenticate should never call the comparator with an empty password. Please report.\n"); + return false; + } + return !strcmp(password, hash); } @@ -2799,12 +2814,12 @@ static const char *RCon_Authenticate(lhnetaddress_t *peeraddress, const char *pa { have_usernames = true; strlcpy(buf, userpass_start, ((size_t)(userpass_end-userpass_start) >= sizeof(buf)) ? (int)(sizeof(buf)) : (int)(userpass_end-userpass_start+1)); - if(buf[0]) + if(buf[0]) // Ignore empty entries due to leading/duplicate space. if(comparator(peeraddress, buf, password, cs, cslen)) goto allow; userpass_start = userpass_end + 1; } - if(userpass_start[0]) + if(userpass_start[0]) // Ignore empty trailing entry due to trailing space or password not set. { userpass_end = userpass_start + strlen(userpass_start); if(comparator(peeraddress, userpass_start, password, cs, cslen)) @@ -2818,12 +2833,12 @@ static const char *RCon_Authenticate(lhnetaddress_t *peeraddress, const char *pa { have_usernames = true; strlcpy(buf, userpass_start, ((size_t)(userpass_end-userpass_start) >= sizeof(buf)) ? (int)(sizeof(buf)) : (int)(userpass_end-userpass_start+1)); - if(buf[0]) + if(buf[0]) // Ignore empty entries due to leading/duplicate space. if(comparator(peeraddress, buf, password, cs, cslen)) goto check; userpass_start = userpass_end + 1; } - if(userpass_start[0]) + if(userpass_start[0]) // Ignore empty trailing entry due to trailing space or password not set. { userpass_end = userpass_start + strlen(userpass_start); if(comparator(peeraddress, userpass_start, password, cs, cslen)) -- 2.39.2