From cac8846eea03e424678290e12e620aca3d7a6f01 Mon Sep 17 00:00:00 2001 From: Levi Neely <141506390+lneely@users.noreply.github.com> Date: Tue, 11 Mar 2025 14:30:47 +0100 Subject: [PATCH] security enhancements and code cleanup - remove logout2 indirection - wipe on logout and unlink (psynclib) - formatting - rpcclient: wipe request/response buffers before freeing - function renames: "read*" --- main.cpp | 4 ++-- pclsync/pnetlibs.c | 8 ++++---- pclsync/psynclib.c | 32 ++++++++++++++++++-------------- pclsync/psynclib.h | 2 +- pclsync_lib.cpp | 34 ++++++++++++++-------------------- pclsync_lib.h | 11 +++++------ rpcclient.cpp | 5 ++++- 7 files changed, 48 insertions(+), 48 deletions(-) diff --git a/main.cpp b/main.cpp index dc4d972..b81a0e6 100644 --- a/main.cpp +++ b/main.cpp @@ -117,7 +117,7 @@ int main(int argc, char **argv) { cc::clibrary::pclsync_lib::get_lib().set_username(username); if (passwordsw) { - cc::clibrary::pclsync_lib::get_lib().get_pass_from_console(); + cc::clibrary::pclsync_lib::get_lib().read_password(); } cc::clibrary::pclsync_lib::get_lib().set_tfa_code(tfa_code); cc::clibrary::pclsync_lib::get_lib().set_trusted_device(trusted_device); @@ -127,7 +127,7 @@ int main(int argc, char **argv) { cc::clibrary::pclsync_lib::get_lib().set_crypto_pass(password); } else { std::cout << "Enter crypto password." << std::endl; - cc::clibrary::pclsync_lib::get_lib().get_cryptopass_from_console(); + cc::clibrary::pclsync_lib::get_lib().read_cryptopass(); } } else cc::clibrary::pclsync_lib::get_lib().setup_crypto_ = false; diff --git a/pclsync/pnetlibs.c b/pclsync/pnetlibs.c index 1c6e273..a93f63d 100644 --- a/pclsync/pnetlibs.c +++ b/pclsync/pnetlibs.c @@ -2583,14 +2583,14 @@ int psync_get_upload_checksum(psync_uploadid_t uploadid, unsigned char *uhash, return PSYNC_NET_OK; } -void psync_logout2(uint32_t auth_status, int doinvauth); - -static void logout2_thread() { psync_logout2(PSTATUS_AUTH_BADTOKEN, 0); } +static void proc_logout() { + psync_logout(PSTATUS_AUTH_BADTOKEN, 0); +} // this is called when ANY api call returns non zero result void psync_process_api_error(uint64_t result) { if (result == 2000) - prun_thread("logout from process_api_error", logout2_thread); + prun_thread("logout from process_api_error", proc_logout); } static void psync_netlibs_timer(psync_timer_t timer, void *ptr) { diff --git a/pclsync/psynclib.c b/pclsync/psynclib.c index 62ecb0e..c4d6073 100644 --- a/pclsync/psynclib.c +++ b/pclsync/psynclib.c @@ -428,21 +428,25 @@ static void psync_invalidate_auth(const char *auth) { psync_run_command("logout", params, NULL); } -void psync_logout2(uint32_t auth_status, int doinvauth) { +void psync_logout(uint32_t auth_status, int doinvauth) { tfa = 0; debug(D_NOTICE, "logout"); - psync_sql_statement( - "DELETE FROM setting WHERE id IN ('pass', 'auth', 'saveauth')"); - if (doinvauth) + + psync_sql_statement("DELETE FROM setting WHERE id IN ('pass', 'auth', 'saveauth')"); + if (doinvauth) { psync_invalidate_auth(psync_my_auth); - memset(psync_my_auth, 0, sizeof(psync_my_auth)); + } + putil_wipe(psync_my_auth, sizeof(psync_my_auth)); pcryptofolder_lock(); + pthread_mutex_lock(&psync_my_auth_mutex); + putil_wipe(psync_my_pass, sizeof(psync_my_pass)); psync_free(psync_my_pass); - psync_my_pass = NULL; pthread_mutex_unlock(&psync_my_auth_mutex); + pstatus_set(PSTATUS_TYPE_ONLINE, PSTATUS_ONLINE_CONNECTING); pstatus_set(PSTATUS_TYPE_AUTH, auth_status); + psync_fs_pause_until_login(); pdownload_stop_all(); pupload_stop_all(); @@ -451,12 +455,11 @@ void psync_logout2(uint32_t auth_status, int doinvauth) { psync_set_apiserver(PSYNC_API_HOST, PSYNC_LOCATIONID_DEFAULT); psync_restart_localscan(); ptimer_notify_exception(); - if (psync_fs_need_per_folder_refresh()) + if (psync_fs_need_per_folder_refresh()) { psync_fs_refresh_folder(0); + } } -void psync_logout() { psync_logout2(PSTATUS_AUTH_REQUIRED, 1); } - apiservers_list_t *psync_get_apiservers(char **err) { psock_t *api; binresult *bres; @@ -573,19 +576,19 @@ void psync_unlink() { ppagecache_clean(); psync_sql_connect(psync_database); if (deviceid) { - res = psync_sql_prep_statement( - "REPLACE INTO setting (id, value) VALUES ('deviceid', ?)"); + res = psync_sql_prep_statement("REPLACE INTO setting (id, value) VALUES ('deviceid', ?)"); psync_sql_bind_string(res, 1, deviceid); psync_sql_run_free(res); psync_free(deviceid); } pthread_mutex_lock(&psync_my_auth_mutex); - memset(psync_my_auth, 0, sizeof(psync_my_auth)); + putil_wipe(psync_my_auth, sizeof(psync_my_auth)); psync_my_user = NULL; - psync_my_pass = NULL; + putil_wipe(psync_my_pass, sizeof(psync_my_pass)); psync_my_userid = 0; pthread_mutex_unlock(&psync_my_auth_mutex); debug(D_NOTICE, "clearing database, finished"); + psync_fs_pause_until_login(); psync_fs_clean_tasks(); ppathstatus_init(); @@ -602,8 +605,9 @@ void psync_unlink() { pstatus_set(PSTATUS_TYPE_AUTH, PSTATUS_AUTH_REQUIRED); pstatus_set(PSTATUS_TYPE_RUN, PSTATUS_RUN_RUN); psync_resume_localscan(); - if (psync_fs_need_per_folder_refresh()) + if (psync_fs_need_per_folder_refresh()) { psync_fs_refresh_folder(0); + } } int psync_tfa_has_devices() { return psync_my_2fa_has_devices; } diff --git a/pclsync/psynclib.h b/pclsync/psynclib.h index 4f7cb55..095899d 100644 --- a/pclsync/psynclib.h +++ b/pclsync/psynclib.h @@ -638,7 +638,7 @@ char *psync_get_username(); void psync_set_user_pass(const char *username, const char *password, int save); void psync_set_pass(const char *password, int save); void psync_set_auth(const char *auth, int save); -void psync_logout(); +void psync_logout(uint32_t auth_status, int doinvauth); void psync_unlink(); /* Upon seein a status of PSTATUS_TFA_REQUIRED the application is supposed to diff --git a/pclsync_lib.cpp b/pclsync_lib.cpp index 77a1df0..72152d5 100644 --- a/pclsync_lib.cpp +++ b/pclsync_lib.cpp @@ -94,9 +94,6 @@ void clib::pclsync_lib::set_tfa_code(const std::string &arg) { void clib::pclsync_lib::set_username(const std::string &arg) { username_ = arg; } -void clib::pclsync_lib::set_password(const std::string &arg) { - password_ = arg; -} void clib::pclsync_lib::set_crypto_pass(const std::string &arg) { crypto_pass_ = arg; }; @@ -116,30 +113,27 @@ clib::pclsync_lib &clib::pclsync_lib::get_lib() { char *clib::pclsync_lib::get_token() { return psync_get_token(); } -void clib::pclsync_lib::get_pass_from_console() { - do_get_pass_from_console(password_); +void clib::pclsync_lib::read_password() { + read_from_stdin(password_); } -void clib::pclsync_lib::get_tfa_code_from_console() +void clib::pclsync_lib::read_tfa_code() { if (daemon_) { - std::cout << "Not able to read 2fa code when started as daemon." - << std::endl; + std::cout << "Not able to read 2fa code when started as daemon." << std::endl; exit(1); } - std::cout << "Please enter 2fa code" - << std::endl; + std::cout << "Please enter 2fa code" << std::endl; getline(std::cin, tfa_code_); } -void clib::pclsync_lib::get_cryptopass_from_console() { - do_get_pass_from_console(crypto_pass_); +void clib::pclsync_lib::read_cryptopass() { + read_from_stdin(crypto_pass_); } -void clib::pclsync_lib::do_get_pass_from_console(std::string &password) { +void clib::pclsync_lib::read_from_stdin(std::string &s) { if (daemon_) { - std::cout << "Not able to read password when started as daemon." - << std::endl; + std::cout << "Not able to read password when started as daemon." << std::endl; exit(1); } termios oldt; @@ -148,7 +142,7 @@ void clib::pclsync_lib::do_get_pass_from_console(std::string &password) { newt.c_lflag &= ~ECHO; tcsetattr(STDIN_FILENO, TCSANOW, &newt); std::cout << "Please, enter password" << std::endl; - getline(std::cin, password); + getline(std::cin, s); tcsetattr(STDIN_FILENO, TCSANOW, &oldt); } @@ -302,7 +296,7 @@ static void status_change(pstatus_t *status) { *clib::pclsync_lib::get_lib().status_ = *status; if (status->status == PSTATUS_LOGIN_REQUIRED) { if (clib::pclsync_lib::get_lib().get_password().empty()) { - clib::pclsync_lib::get_lib().get_pass_from_console(); + clib::pclsync_lib::get_lib().read_password(); } psync_set_user_pass(clib::pclsync_lib::get_lib().get_username().c_str(), @@ -311,7 +305,7 @@ static void status_change(pstatus_t *status) { std::cout << "logging in" << std::endl; } else if (status->status == PSTATUS_TFA_REQUIRED) { if (clib::pclsync_lib::get_lib().get_tfa_code().empty()) { - clib::pclsync_lib::get_lib().get_tfa_code_from_console(); + clib::pclsync_lib::get_lib().read_tfa_code(); } psync_tfa_set_code(clib::pclsync_lib::get_lib().get_tfa_code().c_str(), @@ -319,7 +313,7 @@ static void status_change(pstatus_t *status) { 0); } else if (status->status == PSTATUS_BAD_LOGIN_DATA) { if (!clib::pclsync_lib::get_lib().newuser_) { - clib::pclsync_lib::get_lib().get_pass_from_console(); + clib::pclsync_lib::get_lib().read_password(); psync_set_user_pass(clib::pclsync_lib::get_lib().get_username().c_str(), clib::pclsync_lib::get_lib().get_password().c_str(), (int)clib::pclsync_lib::get_lib().save_pass_); @@ -495,7 +489,7 @@ int clib::pclsync_lib::login(const char *user, const char *pass, int save) { int clib::pclsync_lib::logout() { wipe_password(); - psync_logout(); + psync_logout(PSTATUS_AUTH_REQUIRED, 1); return 0; } diff --git a/pclsync_lib.h b/pclsync_lib.h index 8c78716..ca1faf7 100644 --- a/pclsync_lib.h +++ b/pclsync_lib.h @@ -68,7 +68,6 @@ public: void set_trusted_device(bool arg); void set_tfa_code(const std::string& arg); void set_username(const std::string &arg); - void set_password(const std::string &arg); void set_crypto_pass(const std::string &arg); void set_mount(const std::string &arg); void set_savepass(bool s); @@ -87,9 +86,9 @@ public: static pclsync_lib &get_lib(); // Console - void get_tfa_code_from_console(); - void get_pass_from_console(); - void get_cryptopass_from_console(); + void read_tfa_code(); + void read_password(); + void read_cryptopass(); // API calls int init(); @@ -118,8 +117,8 @@ private: bool to_set_mount_; bool daemon_; - void do_get_pass_from_console(std::string &password); - void wipe(std::string& str); + void read_from_stdin(std::string &s); + void wipe(std::string& s); }; } // namespace clibrary } // namespace console_client diff --git a/rpcclient.cpp b/rpcclient.cpp index b408be9..5641db4 100644 --- a/rpcclient.cpp +++ b/rpcclient.cpp @@ -11,6 +11,7 @@ #include "rpcclient.h" #include "plibs.h" #include "prpc.h" +#include "putil.h" #define POVERLAY_BUFSIZE 512 @@ -97,12 +98,12 @@ int RpcClient::writeRequest(int fd, int msgtype, const char *value, char **out, writeerr = POVERLAY_WRITE_COMM_ERR; } + putil_wipe(buf, size); free(buf); return writeerr; } int RpcClient::readResponse(int fd, char **out, size_t *out_size) { - rpc_message_t *msg = (rpc_message_t *)malloc(POVERLAY_BUFSIZE); if (msg == NULL) { const char *error_msg = "Memory allocation failed"; @@ -116,6 +117,7 @@ int RpcClient::readResponse(int fd, char **out, size_t *out_size) { const char *error_msg = (bytes_read == 0) ? "Connection closed" : "Read error"; *out = strdup(error_msg); *out_size = strlen(error_msg) + 1; + putil_wipe(msg, POVERLAY_BUFSIZE); free(msg); return -1; } @@ -124,6 +126,7 @@ int RpcClient::readResponse(int fd, char **out, size_t *out_size) { memcpy(*out, msg->value, msg->length); *out_size = msg->length; + putil_wipe(msg, POVERLAY_BUFSIZE); free(msg); return 0; }