Skip to content

Commit f3b61c6

Browse files
lib/, src/: Use getpassa()/passzero() instead of agetpass()/erase_pass()
And getpassa_stdin() instead of agetpass_stdin(). Now our passwords live in the stack, and there are less copies in the heap. In a few programs, we still copy them into the heap, though. It's not easy to get rid of all of them. This alloca(3)-based API means we need to call passalloca() before a loop. Calling passalloca() (which is a wrapper around alloca(3)) in a loop is dangerous, as it can trigger a stack overflow. Instead, allocate the buffer before the loop, and run getpass2() within the loop, which will reuse the buffer. Also, to avoid deallocator mismatches, use `pass = passzero(pass)` in those cases, so that the compiler knows that 'pass' has changed, and we're not using the password after zeroing it; we're only re-using its storage, which is fine. Signed-off-by: Alejandro Colomar <[email protected]>
1 parent ad47118 commit f3b61c6

File tree

5 files changed

+65
-58
lines changed

5 files changed

+65
-58
lines changed

lib/pwauth.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
#include <sys/types.h>
2020
#include <unistd.h>
2121

22-
#include "agetpass.h"
2322
#include "defines.h"
23+
#include "pass.h"
2424
#include "prototypes.h"
2525
#include "pwauth.h"
2626
#include "getdef.h"
@@ -53,7 +53,7 @@ pw_auth(const char *cipher, const char *user)
5353
{
5454
int retval;
5555
char prompt[1024];
56-
char *clear;
56+
pass_t *clear;
5757
const char *cp;
5858
const char *encrypted;
5959
const char *input;
@@ -112,8 +112,8 @@ pw_auth(const char *cipher, const char *user)
112112
#endif
113113

114114
SNPRINTF(prompt, cp, user);
115-
clear = agetpass(prompt);
116-
input = (clear == NULL) ? "" : clear;
115+
clear = getpassa(prompt);
116+
input = (clear == NULL) ? "" : *clear;
117117

118118
/*
119119
* Convert the cleartext password into a ciphertext string.
@@ -138,9 +138,9 @@ pw_auth(const char *cipher, const char *user)
138138
* -- AR 8/22/1999
139139
*/
140140
if ((0 != retval) && streq(input, "") && use_skey) {
141-
erase_pass(clear);
142-
clear = agetpass(prompt);
143-
input = (clear == NULL) ? "" : clear;
141+
passzero(clear);
142+
clear = getpassa(prompt);
143+
input = (clear == NULL) ? "" : *clear;
144144
}
145145

146146
if ((0 != retval) && use_skey) {
@@ -154,7 +154,7 @@ pw_auth(const char *cipher, const char *user)
154154
}
155155
}
156156
#endif
157-
erase_pass(clear);
157+
passzero(clear);
158158

159159
return retval;
160160
}

src/gpasswd.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@
2020
#include <stdio.h>
2121
#include <sys/types.h>
2222

23-
#include "agetpass.h"
2423
#include "alloc/x/xmalloc.h"
2524
#include "attr.h"
2625
#include "defines.h"
2726
/*@-exitarg@*/
2827
#include "exitcodes.h"
2928
#include "groupio.h"
3029
#include "nscd.h"
30+
#include "pass.h"
3131
#include "prototypes.h"
3232
#ifdef SHADOWGRP
3333
#include "sgroupio.h"
@@ -792,6 +792,7 @@ static void change_passwd (struct group *gr)
792792
char *cp;
793793
static char pass[PASS_MAX + 1];
794794
int retries;
795+
pass_t *p;
795796
const char *salt;
796797

797798
/*
@@ -802,26 +803,27 @@ static void change_passwd (struct group *gr)
802803
*/
803804
printf (_("Changing the password for group %s\n"), group);
804805

806+
p = passalloca();
805807
for (retries = 0; retries < RETRIES; retries++) {
806-
cp = agetpass (_("New Password: "));
807-
if (NULL == cp) {
808+
p = getpass2(p, _("New Password: "));
809+
if (NULL == p) {
808810
exit (1);
809811
}
810812

811-
STRTCPY(pass, cp);
812-
erase_pass (cp);
813-
cp = agetpass (_("Re-enter new password: "));
814-
if (NULL == cp) {
813+
STRTCPY(pass, *p);
814+
p = passzero(p);
815+
p = getpass2(p, _("Re-enter new password: "));
816+
if (NULL == p) {
815817
MEMZERO(pass);
816818
exit (1);
817819
}
818820

819-
if (streq(pass, cp)) {
820-
erase_pass (cp);
821+
if (streq(pass, *p)) {
822+
passzero(p);
821823
break;
822824
}
823825

824-
erase_pass (cp);
826+
p = passzero(p);
825827
MEMZERO(pass);
826828

827829
if (retries + 1 < RETRIES) {

src/newgrp.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@
1616
#include <stdio.h>
1717
#include <sys/types.h>
1818

19-
#include "agetpass.h"
2019
#include "alloc/x/xmalloc.h"
2120
#include "chkname.h"
2221
#include "defines.h"
2322
/*@-exitarg@*/
2423
#include "exitcodes.h"
2524
#include "getdef.h"
25+
#include "pass.h"
2626
#include "prototypes.h"
2727
#include "search/l/lfind.h"
2828
#include "search/l/lsearch.h"
@@ -124,8 +124,8 @@ static void check_perms (const struct group *grp,
124124
const char *groupname)
125125
{
126126
bool needspasswd = false;
127+
pass_t *p;
127128
struct spwd *spwd;
128-
char *cp;
129129
const char *cpasswd;
130130

131131
/*
@@ -168,8 +168,8 @@ static void check_perms (const struct group *grp,
168168
* get the password from her, and set the salt for
169169
* the decryption from the group file.
170170
*/
171-
cp = agetpass (_("Password: "));
172-
if (NULL == cp) {
171+
p = getpassa(_("Password: "));
172+
if (NULL == p) {
173173
goto failure;
174174
}
175175

@@ -178,8 +178,8 @@ static void check_perms (const struct group *grp,
178178
* password in the group file. The result of this encryption
179179
* must match the previously encrypted value in the file.
180180
*/
181-
cpasswd = pw_encrypt (cp, grp->gr_passwd);
182-
erase_pass (cp);
181+
cpasswd = pw_encrypt(*p, grp->gr_passwd);
182+
passzero(p);
183183

184184
if (NULL == cpasswd) {
185185
fprintf (stderr,

src/passwd.c

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@
2020
#include <sys/types.h>
2121
#include <time.h>
2222

23-
#include "agetpass.h"
2423
#include "atoi/a2i/a2s.h"
2524
#include "chkname.h"
2625
#include "defines.h"
2726
#include "getdef.h"
2827
#include "nscd.h"
28+
#include "pass.h"
2929
#include "prototypes.h"
3030
#include "pwauth.h"
3131
#include "pwio.h"
@@ -179,10 +179,11 @@ usage (int status)
179179
*/
180180
static int new_password (const struct passwd *pw)
181181
{
182-
char *clear; /* Pointer to clear text */
183182
char *cipher; /* Pointer to cipher text */
183+
char *cp;
184184
const char *salt; /* Pointer to new salt */
185-
char *cp; /* Pointer to agetpass() response */
185+
pass_t *clear; /* Pointer to clear text */
186+
pass_t *p; /* Pointer to getpassa() response */
186187
char orig[PASS_MAX + 1]; /* Original password */
187188
char pass[PASS_MAX + 1]; /* New password */
188189
int i; /* Counter for retries */
@@ -197,15 +198,15 @@ static int new_password (const struct passwd *pw)
197198
*/
198199

199200
if (!amroot && !streq(crypt_passwd, "")) {
200-
clear = agetpass (_("Old password: "));
201+
clear = getpassa(_("Old password: "));
201202
if (NULL == clear) {
202203
return -1;
203204
}
204205

205-
cipher = pw_encrypt (clear, crypt_passwd);
206+
cipher = pw_encrypt(*clear, crypt_passwd);
206207

207208
if (NULL == cipher) {
208-
erase_pass (clear);
209+
passzero(clear);
209210
fprintf (stderr,
210211
_("%s: failed to crypt password with previous salt: %s\n"),
211212
Prog, strerror (errno));
@@ -216,7 +217,7 @@ static int new_password (const struct passwd *pw)
216217
}
217218

218219
if (!streq(cipher, crypt_passwd)) {
219-
erase_pass (clear);
220+
passzero(clear);
220221
strzero (cipher);
221222
SYSLOG ((LOG_WARN, "incorrect password for %s",
222223
pw->pw_name));
@@ -226,8 +227,8 @@ static int new_password (const struct passwd *pw)
226227
pw->pw_name);
227228
return -1;
228229
}
229-
STRTCPY(orig, clear);
230-
erase_pass (clear);
230+
STRTCPY(orig, *clear);
231+
passzero(clear);
231232
strzero (cipher);
232233
} else {
233234
strcpy(orig, "");
@@ -281,31 +282,32 @@ static int new_password (const struct passwd *pw)
281282
/*
282283
* root is setting the passphrase from stdin
283284
*/
284-
cp = agetpass_stdin ();
285-
if (NULL == cp) {
285+
p = getpassa_stdin();
286+
if (NULL == p) {
286287
return -1;
287288
}
288-
ret = STRTCPY (pass, cp);
289-
erase_pass (cp);
289+
ret = STRTCPY(pass, *p);
290+
passzero(p);
290291
if (ret == -1) {
291292
(void) fputs (_("Password is too long.\n"), stderr);
292293
MEMZERO(pass);
293294
return -1;
294295
}
295296
} else {
296297
warned = false;
298+
p = passalloca();
297299
for (i = getdef_num ("PASS_CHANGE_TRIES", 5); i > 0; i--) {
298-
cp = agetpass (_("New password: "));
299-
if (NULL == cp) {
300+
p = getpass2(p, _("New password: "));
301+
if (NULL == p) {
300302
MEMZERO(orig);
301303
MEMZERO(pass);
302304
return -1;
303305
}
304-
if (warned && !streq(pass, cp)) {
306+
if (warned && !streq(pass, *p)) {
305307
warned = false;
306308
}
307-
ret = STRTCPY (pass, cp);
308-
erase_pass (cp);
309+
ret = STRTCPY(pass, *p);
310+
p = passzero(p);
309311
if (ret == -1) {
310312
(void) fputs (_("Password is too long.\n"), stderr);
311313
MEMZERO(orig);
@@ -329,17 +331,17 @@ static int new_password (const struct passwd *pw)
329331
warned = true;
330332
continue;
331333
}
332-
cp = agetpass (_("Re-enter new password: "));
333-
if (NULL == cp) {
334+
p = getpass2(p, _("Re-enter new password: "));
335+
if (NULL == p) {
334336
MEMZERO(orig);
335337
MEMZERO(pass);
336338
return -1;
337339
}
338-
if (!streq(cp, pass)) {
339-
erase_pass (cp);
340+
if (!streq(*p, pass)) {
341+
p = passzero(p);
340342
(void) fputs (_("They don't match; try again.\n"), stderr);
341343
} else {
342-
erase_pass (cp);
344+
passzero(p);
343345
break;
344346
}
345347
}
@@ -1088,12 +1090,14 @@ main(int argc, char **argv)
10881090
*/
10891091
if (!anyflag && use_pam) {
10901092
if (sflg) {
1091-
cp = agetpass_stdin ();
1092-
if (cp == NULL) {
1093+
pass_t *p;
1094+
1095+
p = getpassa_stdin();
1096+
if (p == NULL) {
10931097
exit (E_FAILURE);
10941098
}
1095-
do_pam_passwd_non_interactive ("passwd", name, cp);
1096-
erase_pass (cp);
1099+
do_pam_passwd_non_interactive ("passwd", name, *p);
1100+
passzero(p);
10971101
} else {
10981102
do_pam_passwd (name, qflg, kflg);
10991103
}

src/sulogin.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@
1818
#include <sys/ioctl.h>
1919
#include <sys/types.h>
2020

21-
#include "agetpass.h"
2221
#include "attr.h"
2322
#include "defines.h"
2423
#include "getdef.h"
24+
#include "pass.h"
2525
#include "prototypes.h"
2626
#include "pwauth.h"
2727
/*@-exitarg@*/
@@ -59,6 +59,7 @@ int
5959
main(int argc, char *argv[])
6060
{
6161
int err = 0;
62+
pass_t *pass;
6263
char **envp = environ;
6364
TERMIO termio;
6465
struct passwd pwent = {};
@@ -129,8 +130,8 @@ main(int argc, char *argv[])
129130
(void) signal (SIGALRM, catch_signals); /* exit if the timer expires */
130131
(void) alarm (ALARM); /* only wait so long ... */
131132

133+
pass = passalloca();
132134
do { /* repeatedly get login/password pairs */
133-
char *pass;
134135
const char *prompt;
135136

136137
if (pw_entry("root", &pwent) == -1) { /* get entry from password file */
@@ -151,25 +152,25 @@ main(int argc, char *argv[])
151152
"(or give root password for system maintenance):");
152153

153154
/* get a password for root */
154-
pass = agetpass(prompt);
155+
pass = getpass2(pass, prompt);
155156

156157
/*
157158
* XXX - can't enter single user mode if root password is
158159
* empty. I think this doesn't happen very often :-). But
159160
* it will work with standard getpass() (no NULL on EOF).
160161
* --marekm
161162
*/
162-
if ((NULL == pass) || streq(pass, "")) {
163-
erase_pass (pass);
163+
if (NULL == pass || streq(*pass, "")) {
164+
passzero(pass);
164165
(void) puts ("");
165166
#ifdef TELINIT
166167
execl (PATH_TELINIT, "telinit", RUNLEVEL, (char *) NULL);
167168
#endif
168169
exit (0);
169170
}
170171

171-
done = valid(pass, &pwent);
172-
erase_pass (pass);
172+
done = valid(*pass, &pwent);
173+
pass = passzero(pass);
173174

174175
if (!done) { /* check encrypted passwords ... */
175176
/* ... encrypted passwords did not match */

0 commit comments

Comments
 (0)