Skip to content

Commit 768ade8

Browse files
pam: get username once (#974)
* pam: fix username race by using getpwuid_r instead of getpwuid getpwuid() returns a pointer into a static buffer shared across all threads. Any getpw*/getpwent call from another thread — including those made internally by PAM modules during authentication — will overwrite it before pam_start() reads pw_name, causing hyprlock to authenticate as a random system user (root, bin, systemd-network) or fail with 'user unknown'. Replace with getpwuid_r(), which writes into a caller-supplied buffer, and copy pw_name into a std::string before calling pam_start(). * pam: get username once Instead of retrieving the username via getpwuid_r as in a69f526, get the username once when initializing CPam and save it in a string. This should be sufficent for making sure there are no problems with the static buffer returned by getpwuid and is simpler. * misc: clang-format --------- Co-authored-by: mcgi5sr2 <[email protected]>
1 parent d7079a1 commit 768ade8

5 files changed

Lines changed: 35 additions & 16 deletions

File tree

src/auth/Pam.cpp

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
#include "Pam.hpp"
2+
#include "../config/ConfigManager.hpp"
23
#include "../core/hyprlock.hpp"
34
#include "../helpers/Log.hpp"
4-
#include "../config/ConfigManager.hpp"
5+
#include "../helpers/MiscFunctions.hpp"
56

67
#include <filesystem>
78
#include <unistd.h>
8-
#include <pwd.h>
99
#include <security/pam_appl.h>
1010
#if __has_include(<security/pam_misc.h>)
1111
#include <security/pam_misc.h>
@@ -69,6 +69,8 @@ CPam::CPam() {
6969
m_sPamModule = "su";
7070
}
7171

72+
m_username = getUsernameForCurrentUid();
73+
7274
m_sConversationState.waitForInput = [this]() { this->waitForInput(); };
7375
}
7476

@@ -106,12 +108,15 @@ void CPam::init() {
106108
}
107109

108110
bool CPam::auth() {
109-
const pam_conv localConv = {.conv = conv, .appdata_ptr = (void*)&m_sConversationState};
110-
pam_handle_t* handle = nullptr;
111-
auto uidPassword = getpwuid(getuid());
112-
RASSERT(uidPassword && uidPassword->pw_name, "Failed to get username (getpwuid)");
111+
const pam_conv localConv = {.conv = conv, .appdata_ptr = (void*)&m_sConversationState};
112+
pam_handle_t* handle = nullptr;
113+
114+
if (m_username.empty()) {
115+
m_sConversationState.failText = "Username not set";
116+
return false;
117+
}
113118

114-
int ret = pam_start(m_sPamModule.c_str(), uidPassword->pw_name, &localConv, &handle);
119+
int ret = pam_start(m_sPamModule.c_str(), m_username.c_str(), &localConv, &handle);
115120

116121
if (ret != PAM_SUCCESS) {
117122
m_sConversationState.failText = "pam_start failed";

src/auth/Pam.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ class CPam : public IAuthImplementation {
4646

4747
bool m_bBlockInput = true;
4848

49-
std::string m_sPamModule;
49+
std::string m_sPamModule = "";
50+
std::string m_username = "";
5051

5152
bool auth();
5253
void resetConversation();

src/config/ConfigManager.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -369,11 +369,10 @@ std::vector<CConfigManager::SWidgetConfig> CConfigManager::getWidgetConfigs() {
369369

370370
#define SHADOWABLE(name) \
371371
{"shadow_size", m_config.getSpecialConfigValue(name, "shadow_size", k.c_str())}, {"shadow_passes", m_config.getSpecialConfigValue(name, "shadow_passes", k.c_str())}, \
372-
{"shadow_color", m_config.getSpecialConfigValue(name, "shadow_color", k.c_str())}, { \
373-
"shadow_boost", m_config.getSpecialConfigValue(name, "shadow_boost", k.c_str()) \
374-
}
372+
{"shadow_color", m_config.getSpecialConfigValue(name, "shadow_color", k.c_str())}, {"shadow_boost", m_config.getSpecialConfigValue(name, "shadow_boost", k.c_str())}
375373

376-
#define CLICKABLE(name) {"onclick", m_config.getSpecialConfigValue(name, "onclick", k.c_str())}
374+
#define CLICKABLE(name) \
375+
{ "onclick", m_config.getSpecialConfigValue(name, "onclick", k.c_str()) }
377376

378377
//
379378
auto keys = m_config.listKeysForSpecialCategory("background");

src/helpers/MiscFunctions.cpp

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
#include <filesystem>
1+
#include "MiscFunctions.hpp"
2+
#include "Log.hpp"
3+
24
#include <algorithm>
35
#include <cmath>
46
#include <fcntl.h>
5-
#include "MiscFunctions.hpp"
6-
#include "Log.hpp"
7-
#include <hyprutils/string/String.hpp>
7+
#include <filesystem>
88
#include <hyprutils/os/Process.hpp>
9+
#include <hyprutils/string/String.hpp>
10+
#include <pwd.h>
911
#include <unistd.h>
1012

1113
using namespace Hyprutils::String;
@@ -158,3 +160,14 @@ void spawnAsync(const std::string& cmd) {
158160
if (!proc.runAsync())
159161
Debug::log(ERR, "Failed to start \"{}\"", cmd);
160162
}
163+
164+
std::string getUsernameForCurrentUid() {
165+
const uid_t UID = getuid();
166+
auto uidPassword = getpwuid(UID);
167+
if (!uidPassword || !uidPassword->pw_name) {
168+
Debug::log(ERR, "Failed to get username for uid {} (getpwuid)", UID);
169+
return "";
170+
}
171+
172+
return std::string{uidPassword->pw_name};
173+
}

src/helpers/MiscFunctions.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ int64_t configStringToInt(const std::string& VALUE);
99
int createPoolFile(size_t size, std::string& name);
1010
std::string spawnSync(const std::string& cmd);
1111
void spawnAsync(const std::string& cmd);
12+
std::string getUsernameForCurrentUid();

0 commit comments

Comments
 (0)