Commit a33ec22b authored by Fabian Vogt's avatar Fabian Vogt

Several cleanups

Summary:
- No cppcheck warnings anymore
- Use snprintf everywhere
- Avoid pointless multiplication with sizeof(char)
- Avoid memory leaks

Test Plan: Still builds, works the same as before.

Reviewers: #plasma

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D7123
parent 38ff660b
......@@ -151,13 +151,14 @@ static int set_env(pam_handle_t *pamh, const char *name, const char *value)
//We do not return because pam_putenv might work
}
char *pamEnv = malloc(strlen(name) + strlen(value) + 2); //2 is for = and \0
size_t pamEnvSize = strlen(name) + strlen(value) + 2; //2 is for = and \0
char *pamEnv = malloc(pamEnvSize);
if (!pamEnv) {
pam_syslog(pamh, LOG_WARNING, "%s: Impossible to allocate memory for pamEnv", logPrefix);
return -1;
}
sprintf (pamEnv, "%s=%s", name, value);
snprintf (pamEnv, pamEnvSize, "%s=%s", name, value);
int ret = pam_putenv(pamh, pamEnv);
free(pamEnv);
......@@ -240,6 +241,11 @@ cleanup:
return result;
}
static void cleanup_free(pam_handle_t *pamh, void *ptr, int error_status)
{
free(ptr);
}
PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **argv)
{
pam_syslog(pamh, LOG_INFO, "%s: pam_sm_authenticate\n", logPrefix);
......@@ -297,14 +303,17 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, cons
return PAM_IGNORE;
}
char *key = malloc(sizeof(char) * KWALLET_PAM_KEYSIZE);
if (kwallet_hash(password, userInfo, key) != 0) {
char *key = malloc(KWALLET_PAM_KEYSIZE);
if (!key || kwallet_hash(password, userInfo, key) != 0) {
free(key);
pam_syslog(pamh, LOG_ERR, "%s: Fail into creating the hash", logPrefix);
return PAM_IGNORE;
}
result = pam_set_data(pamh, kwalletPamDataKey, key, NULL);
result = pam_set_data(pamh, kwalletPamDataKey, key, cleanup_free);
if (result != PAM_SUCCESS) {
free(key);
pam_syslog(pamh, LOG_ERR, "%s: Impossible to store the hashed password: %s", logPrefix
, pam_strerror(pamh, result));
return PAM_IGNORE;
......@@ -385,9 +394,8 @@ cleanup:
static int better_write(int fd, const char *buffer, int len)
{
size_t writtenBytes = 0;
int result;
while(writtenBytes < len) {
result = write(fd, buffer + writtenBytes, len - writtenBytes);
int result = write(fd, buffer + writtenBytes, len - writtenBytes);
if (result < 0) {
if (errno != EAGAIN && errno != EINTR) {
return -1;
......@@ -450,6 +458,7 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha
if (result != PAM_SUCCESS) {
pam_syslog(pamh, LOG_ERR, "%s: Impossible to set %s env, %s",
logPrefix, envVar, pam_strerror(pamh, result));
free(fullSocket);
return;
}
......@@ -459,12 +468,15 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha
if (strlen(fullSocket) > sizeof(local.sun_path)) {
pam_syslog(pamh, LOG_ERR, "%s: socket path %s too long to open",
logPrefix, fullSocket);
free(fullSocket);
return;
}
strcpy(local.sun_path, fullSocket);
free(fullSocket);
fullSocket = NULL;
unlink(local.sun_path);//Just in case it exists from a previous login
pam_syslog(pamh, LOG_INFO, "%s: final socket path: %s", logPrefix, fullSocket);
pam_syslog(pamh, LOG_INFO, "%s: final socket path: %s", logPrefix, local.sun_path);
size_t len = strlen(local.sun_path) + sizeof(local.sun_family);
if (bind(envSocket, (struct sockaddr *)&local, len) == -1) {
......@@ -477,7 +489,7 @@ static void start_kwallet(pam_handle_t *pamh, struct passwd *userInfo, const cha
return;
}
if (chown(fullSocket, userInfo->pw_uid, userInfo->pw_gid) == -1) {
if (chown(local.sun_path, userInfo->pw_uid, userInfo->pw_gid) == -1) {
pam_syslog(pamh, LOG_INFO, "%s: Couldn't change ownership of the socket", logPrefix);
return;
}
......@@ -655,7 +667,8 @@ int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
#else
char *fixpath = "share/apps/kwallet/kdewallet.salt";
#endif
char *path = (char*) malloc(strlen(userInfo->pw_dir) + strlen(kdehome) + strlen(fixpath) + 3);//3 == / and \0
size_t pathSize = strlen(userInfo->pw_dir) + strlen(kdehome) + strlen(fixpath) + 3;//3 == /, / and \0
char *path = (char*) malloc(pathSize);
sprintf(path, "%s/%s/%s", userInfo->pw_dir, kdehome, fixpath);
struct stat info;
......@@ -666,21 +679,26 @@ int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
FILE *fd = fopen(path, "r");
if (fd == NULL) {
syslog(LOG_ERR, "%s: Couldn't open file: %s because: %d-%s", logPrefix, path, errno, strerror(errno));
free(path);
return 1;
}
salt = (char*) malloc(sizeof(char) * KWALLET_PAM_SALTSIZE);
salt = (char*) malloc(KWALLET_PAM_SALTSIZE);
memset(salt, '\0', KWALLET_PAM_SALTSIZE);
fread(salt, KWALLET_PAM_SALTSIZE, 1, fd);
fclose(fd);
}
free(path);
if (salt == NULL) {
syslog(LOG_ERR, "%s-kwalletd: Couldn't create or read the salt file", logPrefix);
return 1;
}
gcry_error_t error;
error = gcry_control(GCRYCTL_INIT_SECMEM, 32768, 0);
if (error != 0) {
free(salt);
syslog(LOG_ERR, "%s-kwalletd: Can't get secure memory: %d", logPrefix, error);
return 1;
}
......@@ -691,5 +709,7 @@ int kwallet_hash(const char *passphrase, struct passwd *userInfo, char *key)
GCRY_KDF_PBKDF2, GCRY_MD_SHA512,
salt, KWALLET_PAM_SALTSIZE,
KWALLET_PAM_ITERATIONS,KWALLET_PAM_KEYSIZE, key);
return 0;
free(salt);
return (int) error; // gcry_kdf_derive returns 0 on success
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment