Skip to content

Commit

Permalink
Prevent keeping password in memory after password change.
Browse files Browse the repository at this point in the history
  • Loading branch information
sfence committed Apr 9, 2024
1 parent fbb0c35 commit 47a8ee1
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 13 deletions.
14 changes: 11 additions & 3 deletions src/client/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1335,17 +1335,25 @@ void Client::clearOutChatQueue()
m_out_chat_queue = std::queue<std::wstring>();
}

void Client::sendChangePassword(const std::string &oldpassword,
const std::string &newpassword)
void Client::sendChangePassword(std::string &oldpassword,
std::string &newpassword)
{
LocalPlayer *player = m_env.getLocalPlayer();
if (player == NULL)
if (player == NULL) {
porting::secure_clear_string(oldpassword);
porting::secure_clear_string(newpassword);
return;
}

// get into sudo mode and then send new password to server
std::string playername = m_env.getLocalPlayer()->getName();
m_auth->applyPassword(playername, oldpassword);
m_new_auth.applyPassword(playername, newpassword);

// we do not need to keep passwords in memory
porting::secure_clear_string(oldpassword);
porting::secure_clear_string(newpassword);

startAuth(choseAuthMech(m_sudo_auth_methods));
}

Expand Down
4 changes: 2 additions & 2 deletions src/client/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,8 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
void sendInventoryAction(InventoryAction *a);
void sendChatMessage(const std::wstring &message);
void clearOutChatQueue();
void sendChangePassword(const std::string &oldpassword,
const std::string &newpassword);
void sendChangePassword(std::string &oldpassword,
std::string &newpassword);
void sendDamage(u16 damage);
void sendRespawn();
void sendReady();
Expand Down
10 changes: 9 additions & 1 deletion src/gui/guiPasswordChange.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,20 @@ void GUIPasswordChange::acceptInput()
bool GUIPasswordChange::processInput()
{
if (m_newpass != m_newpass_confirm) {
porting::secure_clear_string(m_oldpass);
porting::secure_clear_string(m_newpass);
porting::secure_clear_string(m_newpass_confirm);
gui::IGUIElement *e = getElementFromId(ID_message);
if (e != NULL)
e->setVisible(true);
return false;
}
m_client->sendChangePassword(wide_to_utf8(m_oldpass), wide_to_utf8(m_newpass));
std::string old_pass = wide_to_utf8(m_oldpass);
std::string new_pass = wide_to_utf8(m_newpass);
porting::secure_clear_string(m_oldpass);
porting::secure_clear_string(m_newpass);
porting::secure_clear_string(m_newpass_confirm);
m_client->sendChangePassword(old_pass, new_pass);
return true;
}

Expand Down
25 changes: 18 additions & 7 deletions src/porting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -891,21 +891,32 @@ double perf_freq = get_perf_freq();

#endif

/// Override every character before clearing
void secure_clear_string(std::string &text)
/// Override every byte before clearing
void secure_clear_memory(volatile void *ptr, size_t size)
{
#ifdef __STDC_LIB_EXT1__
memset_s((void *)text.data(), text.size(), '0', text.size());
memset_s(prt, size, '0', size);
#elif _WIN32
SecureZeroMemory((void *)text.data(), text.size());
SecureZeroMemory(prt, size);
#else
volatile char *ch = (char *)text.data();
size_t n = text.size();
for (;n>0;n--) {
volatile char *ch = (char *)ptr;
for (;size>0;size--) {
*ch = 0;
ch++;
}
#endif
}

/// Override every character before clearing
void secure_clear_string(std::string &text)
{
secure_clear_memory((void *)text.data(), text.size());
text.clear();
}

void secure_clear_string(std::wstring &text)
{
secure_clear_memory((void *)text.data(), text.size()*sizeof(wchar_t));
text.clear();
}

Expand Down
5 changes: 5 additions & 0 deletions src/porting.h
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,13 @@ bool open_url(const std::string &url);
*/
bool open_directory(const std::string &path);


/// Clear memory by overwriting every character (used for security)
void secure_clear_memory(volatile void *ptr, size_t size);

/// Clear string by overwriting every character (used for security)
void secure_clear_string(std::string &text);
void secure_clear_string(std::wstring &text);

} // namespace porting

Expand Down

0 comments on commit 47a8ee1

Please sign in to comment.