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

@@ -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;

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 proc_logout() { 
-
+  psync_logout(PSTATUS_AUTH_BADTOKEN, 0); 
-static void logout2_thread() { psync_logout2(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) {

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')");
+  psync_sql_statement("DELETE FROM setting WHERE id IN ('pass', 'auth', 'saveauth')");
-  if (doinvauth)
+  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(
+    res = psync_sql_prep_statement("REPLACE INTO setting (id, value) VALUES ('deviceid', ?)");
-        "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; }

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

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() {
+void clib::pclsync_lib::read_password() {
-  do_get_pass_from_console(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::cout << "Not able to read 2fa code when started as daemon." << std::endl;
-              << std::endl;
     exit(1);
   }
-  std::cout << "Please enter 2fa code" 
+  std::cout << "Please enter 2fa code" << std::endl;
-            << std::endl;
   getline(std::cin, tfa_code_);
 }
 
-void clib::pclsync_lib::get_cryptopass_from_console() {
+void clib::pclsync_lib::read_cryptopass() {
-  do_get_pass_from_console(crypto_pass_);
+  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::cout << "Not able to read password when started as daemon." << std::endl;
-              << 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;
 }
 

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 read_tfa_code();
-  void get_pass_from_console();
+  void read_password();
-  void get_cryptopass_from_console();
+  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 read_from_stdin(std::string &s);
-  void wipe(std::string& str);
+  void wipe(std::string& s);
 };
 } // namespace clibrary
 } // namespace console_client

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;
 }