From 287b56a35c8cc39fa78aeb65b17a0bf935f534e3 Mon Sep 17 00:00:00 2001 From: singe Date: Thu, 16 Aug 2018 16:47:17 -0400 Subject: [PATCH] strlcpy fixups on sycophant code C how does it work? Fixed up my bad use of strlcpy that was causing segfaults. Spent ages debugging a crazy error, where if I don't use the ident char (the one set to 't' in eap_server.c) it gets set to the location of the state file, which then breaks everything. Debugging got me nowhere, and I suspect it's a crazy compiler optimisation breaking my poor C. Any tips? --- src/eap_server/eap_server.c | 5 +-- src/eap_server/eap_server_mschapv2.c | 46 +++++++++++++++------------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/eap_server/eap_server.c b/src/eap_server/eap_server.c index 1aa1aeda..57cf70b8 100644 --- a/src/eap_server/eap_server.c +++ b/src/eap_server/eap_server.c @@ -180,7 +180,7 @@ int eap_user_get(struct eap_sm *sm, const u8 *identity, size_t identity_len, if (mana.conf->enable_sycophant && os_strcmp("NOT_SET",mana.conf->sycophant_dir) != 0) { char sup_state[2] = "*"; FILE* sycophantState; - char* sycophantStateFile; + char sycophantStateFile[sizeof(mana.conf->sycophant_dir)+16]; os_strlcpy(sycophantStateFile,mana.conf->sycophant_dir,sizeof(mana.conf->sycophant_dir)); strcat(sycophantStateFile,"SYCOPHANT_STATE"); sycophantState = fopen(sycophantStateFile,"rb"); @@ -190,7 +190,7 @@ int eap_user_get(struct eap_sm *sm, const u8 *identity, size_t identity_len, } if (os_strcmp(sup_state,"I") == 0) { FILE* sycophantID; - char* sycophantIDFile; + char sycophantIDFile[sizeof(mana.conf->sycophant_dir)+15]; os_strlcpy(sycophantIDFile,mana.conf->sycophant_dir,sizeof(mana.conf->sycophant_dir)); if (phase2) strcat(sycophantIDFile,"SYCOPHANT_P2ID"); @@ -209,6 +209,7 @@ int eap_user_get(struct eap_sm *sm, const u8 *identity, size_t identity_len, wpa_printf(MSG_INFO, "MANA EAP Identity Phase %d: %.*s", phase2, (int)identity_len, identity); if (phase2) { char ident = 't'; // This must match the entry in the hostapd.eap_user RADIUS config file + wpa_printf(MSG_DEBUG, "MANA EAP Identiy Phase %d: Setting identity to %c", phase2, ident); identity = (const u8 *)&ident; identity_len = 1; } diff --git a/src/eap_server/eap_server_mschapv2.c b/src/eap_server/eap_server_mschapv2.c index 5fe6e252..6862e458 100644 --- a/src/eap_server/eap_server_mschapv2.c +++ b/src/eap_server/eap_server_mschapv2.c @@ -127,31 +127,35 @@ static struct wpabuf * eap_mschapv2_build_challenge( WPA_PUT_BE16(ms->ms_length, ms_len); //MANA SYCOPHANT START - if (mana.conf->enable_sycophant) { + if (mana.conf->enable_sycophant && os_strcmp("NOT_SET",mana.conf->sycophant_dir) != 0) { char sup_state[2] = "*"; FILE* sycophantState; - char* sycophantStateFile; + char sycophantStateFile[sizeof(mana.conf->sycophant_dir)+16]; os_strlcpy(sycophantStateFile,mana.conf->sycophant_dir,sizeof(mana.conf->sycophant_dir)); strcat(sycophantStateFile,"SYCOPHANT_STATE"); - sycophantState = fopen(sycophantStateFile,"rb"); + wpa_printf(MSG_DEBUG, "SYCOPHANT: Checking Sycophant State File."); while (os_strcmp(sup_state,"C") != 0) { + sycophantState = fopen(sycophantStateFile,"rb"); if (sycophantState == NULL) { wpa_printf (MSG_ERROR,"SYCOPHANT: Unable to open state file %s, not relaying",sycophantStateFile); break; } else { fread(sup_state,1,1,sycophantState); - fclose(sycophantState); if (strcmp(sup_state,"Z") == 0) { + wpa_printf(MSG_DEBUG, "SYCOPHANT: State file is Z bailing!"); + fclose(sycophantState); break; } + fclose(sycophantState); usleep(10000); //Prevent thrashing } } if (strcmp(sup_state,"C") == 0) { + wpa_printf(MSG_DEBUG, "SYCOPHANT: State file says we have a challenge."); FILE* challengeIn; - char* challengeInFile; + char challengeInFile[sizeof(mana.conf->sycophant_dir)+10]; os_strlcpy(challengeInFile,mana.conf->sycophant_dir,sizeof(mana.conf->sycophant_dir)); strcat(challengeInFile,"CHALLENGE"); challengeIn = fopen(challengeInFile, "rb"); @@ -168,11 +172,10 @@ static struct wpabuf * eap_mschapv2_build_challenge( fclose(challengeIn); // Blank file challengeIn = fopen(challengeInFile, "wb"); - fclose(challengeIn); } else { - fclose(challengeIn); usleep(1000); // Prevent thrashing } + fclose(challengeIn); } // TODO: find replace for all these random youtube vids // https://www.youtube.com/watch?v=QUNJ5TRRYqg @@ -357,15 +360,16 @@ static void eap_mschapv2_process_response(struct eap_sm *sm, pos = (u8 *) (resp + 1); //MANA SYCOPHANT START - if (mana.conf->enable_sycophant) { + if (mana.conf->enable_sycophant && os_strcmp("NOT_SET",mana.conf->sycophant_dir) != 0) { char sup_state[2] = "*"; FILE* sycophantState; - char* sycophantStateFile; + char sycophantStateFile[sizeof(mana.conf->sycophant_dir)+16]; os_strlcpy(sycophantStateFile,mana.conf->sycophant_dir,sizeof(mana.conf->sycophant_dir)); strcat(sycophantStateFile,"SYCOPHANT_STATE"); sycophantState = fopen(sycophantStateFile,"rb"); if (sycophantState != NULL) { + wpa_printf(MSG_DEBUG, "SYCOPHANT: Checking state file."); fread(sup_state,1,1,sycophantState); fclose(sycophantState); } else { @@ -373,8 +377,9 @@ static void eap_mschapv2_process_response(struct eap_sm *sm, } if (strcmp(sup_state,"C") == 0) { + wpa_printf(MSG_DEBUG, "SYCOPHANT: State file at Challenge, write the Response."); FILE* responseOut; - char* responseOutFile; + char responseOutFile[sizeof(mana.conf->sycophant_dir)+9]; os_strlcpy(responseOutFile,mana.conf->sycophant_dir,sizeof(mana.conf->sycophant_dir)); strcat(responseOutFile,"RESPONSE"); responseOut = fopen(responseOutFile, "wb"); @@ -384,20 +389,19 @@ static void eap_mschapv2_process_response(struct eap_sm *sm, fwrite(respData->buf,respData->used,1,responseOut); wpa_hexdump(MSG_DEBUG, "SYCOPHANT: Response to be sent to supplicant", respData->buf, respData->used); fclose(responseOut); + // Inform of our readyness + sycophantState = fopen(sycophantStateFile,"wb"); + if (sycophantState != NULL) { + sup_state[0] = 'R'; + fwrite(sup_state,1,1,sycophantState); + fclose(sycophantState); + wpa_printf(MSG_INFO,"SYCOPHANT: MSCHAPv2 Response handed off to supplicant."); + } else { + wpa_printf (MSG_ERROR,"SYCOPHANT: Unable to open state file %s",sycophantStateFile); + } } } - // Inform of our readyness - sycophantState = fopen(sycophantStateFile,"wb"); - - if (sycophantState != NULL) { - sup_state[0] = 'R'; - fwrite(sup_state,1,1,sycophantState); - fclose(sycophantState); - wpa_printf(MSG_INFO,"SYCOPHANT: MSCHAPv2 Response handed off to supplicant."); - } else { - wpa_printf (MSG_ERROR,"SYCOPHANT: Unable to open state file %s",sycophantStateFile); - } } //MANA SYCOPHANT END