Skip to content

Commit cbfce18

Browse files
committed
unixpwd: fix reentrance of unixpwd_setpwd() and unixpwd_setspw()
- prefer fgetpwent_r() over getpwent_r() to avoid ETC_PASSWD FILE to be shared against threads - prefer fgetspent_r() over getspent_r() to avoid ETC_SPASSWD FILE to be shared against threads - while here, unlink(2) tmp_name on error in unixpwd_setpwd() to avoid leaving temporary file on error Signed-off-by: Sebastien Marie <semarie@kapouay.eu.org>
1 parent 7cc153f commit cbfce18

File tree

1 file changed

+28
-10
lines changed

1 file changed

+28
-10
lines changed

unixpwd/c/unixpwd.c

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ unixpwd_setpwd(const char *user, char *password)
8686
*pw;
8787
char buf[BUFLEN];
8888
int tmp;
89-
FILE *tmp_file;
89+
FILE *tmp_file,
90+
*passwd_file;
9091
char tmp_name[PATH_MAX];
9192
struct stat statbuf;
9293
int rc;
@@ -96,18 +97,26 @@ unixpwd_setpwd(const char *user, char *password)
9697
tmp = mkstemp(tmp_name);
9798
if (tmp == -1)
9899
return errno;
99-
if (stat(ETC_PASSWD, &statbuf) != 0
100+
101+
passwd_file = fopen(ETC_PASSWD, "rbe");
102+
if (passwd_file == NULL) {
103+
unlink(tmp_name);
104+
return errno;
105+
}
106+
107+
if (fstat(fileno(passwd_file), &statbuf) != 0
100108
|| fchown(tmp, statbuf.st_uid, statbuf.st_gid) != 0
101109
|| fchmod(tmp, statbuf.st_mode) != 0
102110
|| (tmp_file = fdopen(tmp, "w")) == NULL) {
103111
rc = errno ? errno : EPERM;
112+
fclose(passwd_file);
104113
close(tmp);
114+
unlink(tmp_name);
105115
return rc;
106116
}
107117

108-
setpwent();
109118
while (1) {
110-
rc = getpwent_r(&pwd, buf, BUFLEN, &pw);
119+
rc = fgetpwent_r(passwd_file, &pwd, buf, BUFLEN, &pw);
111120
if (rc != 0 || !pw)
112121
break;
113122
if (!strcmp(user, pw->pw_name)) {
@@ -116,8 +125,8 @@ unixpwd_setpwd(const char *user, char *password)
116125
}
117126
putpwent(pw, tmp_file);
118127
}
119-
endpwent();
120128

129+
fclose(passwd_file);
121130
fclose(tmp_file);
122131
if (rc != ENOENT) {
123132
unlink(tmp_name);
@@ -144,7 +153,8 @@ unixpwd_setspw(const char *user, char *password)
144153
*sp;
145154
char buf[BUFLEN];
146155
int tmp;
147-
FILE *tmp_file = NULL;
156+
FILE *tmp_file = NULL,
157+
*spasswd_file;
148158
char tmp_name[PATH_MAX];
149159
struct stat statbuf;
150160
int rc;
@@ -154,26 +164,34 @@ unixpwd_setspw(const char *user, char *password)
154164
tmp = mkstemp(tmp_name);
155165
if (tmp == -1)
156166
return errno;
157-
if (stat(ETC_SPASSWD, &statbuf) != 0
167+
168+
spasswd_file = fopen(ETC_SPASSWD, "rbe");
169+
if (spasswd_file == NULL) {
170+
unlink(tmp_name);
171+
return errno;
172+
}
173+
174+
if (fstat(fileno(spasswd_file), &statbuf) != 0
158175
|| fchown(tmp, statbuf.st_uid, statbuf.st_gid) != 0
159176
|| fchmod(tmp, statbuf.st_mode) != 0
160177
|| (tmp_file = fdopen(tmp, "w")) == NULL) {
161178
rc = errno ? errno : EPERM;
179+
fclose(spasswd_file);
162180
close(tmp);
163181
unlink(tmp_name);
164182
return rc;
165183
}
166184
if (lckpwdf() != 0) {
185+
fclose(spasswd_file);
167186
if (tmp_file)
168187
fclose(tmp_file);
169188
close(tmp);
170189
unlink(tmp_name);
171190
return ENOLCK;
172191
}
173192

174-
setspent();
175193
while (1) {
176-
rc = getspent_r(&spw, buf, BUFLEN, &sp);
194+
rc = fgetspent_r(spasswd_file, &spw, buf, BUFLEN, &sp);
177195
if (rc != 0 || !sp)
178196
break;
179197
if (!strcmp(user, sp->sp_namp)) {
@@ -182,8 +200,8 @@ unixpwd_setspw(const char *user, char *password)
182200
}
183201
putspent(sp, tmp_file);
184202
}
185-
endspent();
186203

204+
fclose(spasswd_file);
187205
fclose(tmp_file);
188206
if (rc != ENOENT) {
189207
ulckpwdf();

0 commit comments

Comments
 (0)