Skip to content

Commit 213e40d

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 bac1205 commit 213e40d

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"
@@ -820,6 +820,7 @@ static void change_passwd (struct group *gr)
820820
char *cp;
821821
static char pass[PASS_MAX + 1];
822822
int retries;
823+
pass_t *p;
823824
const char *salt;
824825

825826
/*
@@ -830,26 +831,27 @@ static void change_passwd (struct group *gr)
830831
*/
831832
printf (_("Changing the password for group %s\n"), group);
832833

834+
p = passalloca();
833835
for (retries = 0; retries < RETRIES; retries++) {
834-
cp = agetpass (_("New Password: "));
835-
if (NULL == cp) {
836+
p = getpass2(p, _("New Password: "));
837+
if (NULL == p) {
836838
exit (1);
837839
}
838840

839-
STRTCPY(pass, cp);
840-
erase_pass (cp);
841-
cp = agetpass (_("Re-enter new password: "));
842-
if (NULL == cp) {
841+
STRTCPY(pass, *p);
842+
p = passzero(p);
843+
p = getpass2(p, _("Re-enter new password: "));
844+
if (NULL == p) {
843845
MEMZERO(pass);
844846
exit (1);
845847
}
846848

847-
if (streq(pass, cp)) {
848-
erase_pass (cp);
849+
if (streq(pass, *p)) {
850+
passzero(p);
849851
break;
850852
}
851853

852-
erase_pass (cp);
854+
p = passzero(p);
853855
MEMZERO(pass);
854856

855857
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"
@@ -123,8 +123,8 @@ static void check_perms (const struct group *grp,
123123
const char *groupname)
124124
{
125125
bool needspasswd = false;
126+
pass_t *p;
126127
struct spwd *spwd;
127-
char *cp;
128128
const char *cpasswd;
129129

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

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

183183
if (NULL == cpasswd) {
184184
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"
@@ -177,10 +177,11 @@ usage (int status)
177177
*/
178178
static int new_password (const struct passwd *pw)
179179
{
180-
char *clear; /* Pointer to clear text */
181180
char *cipher; /* Pointer to cipher text */
181+
char *cp;
182182
const char *salt; /* Pointer to new salt */
183-
char *cp; /* Pointer to agetpass() response */
183+
pass_t *clear; /* Pointer to clear text */
184+
pass_t *p; /* Pointer to getpassa() response */
184185
char orig[PASS_MAX + 1]; /* Original password */
185186
char pass[PASS_MAX + 1]; /* New password */
186187
int i; /* Counter for retries */
@@ -195,15 +196,15 @@ static int new_password (const struct passwd *pw)
195196
*/
196197

197198
if (!amroot && !streq(crypt_passwd, "")) {
198-
clear = agetpass (_("Old password: "));
199+
clear = getpassa(_("Old password: "));
199200
if (NULL == clear) {
200201
return -1;
201202
}
202203

203-
cipher = pw_encrypt (clear, crypt_passwd);
204+
cipher = pw_encrypt(*clear, crypt_passwd);
204205

205206
if (NULL == cipher) {
206-
erase_pass (clear);
207+
passzero(clear);
207208
fprintf (stderr,
208209
_("%s: failed to crypt password with previous salt: %s\n"),
209210
Prog, strerror (errno));
@@ -214,7 +215,7 @@ static int new_password (const struct passwd *pw)
214215
}
215216

216217
if (!streq(cipher, crypt_passwd)) {
217-
erase_pass (clear);
218+
passzero(clear);
218219
strzero (cipher);
219220
SYSLOG ((LOG_WARN, "incorrect password for %s",
220221
pw->pw_name));
@@ -224,8 +225,8 @@ static int new_password (const struct passwd *pw)
224225
pw->pw_name);
225226
return -1;
226227
}
227-
STRTCPY(orig, clear);
228-
erase_pass (clear);
228+
STRTCPY(orig, *clear);
229+
passzero(clear);
229230
strzero (cipher);
230231
} else {
231232
strcpy(orig, "");
@@ -279,31 +280,32 @@ static int new_password (const struct passwd *pw)
279280
/*
280281
* root is setting the passphrase from stdin
281282
*/
282-
cp = agetpass_stdin ();
283-
if (NULL == cp) {
283+
p = getpassa_stdin();
284+
if (NULL == p) {
284285
return -1;
285286
}
286-
ret = STRTCPY (pass, cp);
287-
erase_pass (cp);
287+
ret = STRTCPY(pass, *p);
288+
passzero(p);
288289
if (ret == -1) {
289290
(void) fputs (_("Password is too long.\n"), stderr);
290291
MEMZERO(pass);
291292
return -1;
292293
}
293294
} else {
294295
warned = false;
296+
p = passalloca();
295297
for (i = getdef_num ("PASS_CHANGE_TRIES", 5); i > 0; i--) {
296-
cp = agetpass (_("New password: "));
297-
if (NULL == cp) {
298+
p = getpass2(p, _("New password: "));
299+
if (NULL == p) {
298300
MEMZERO(orig);
299301
MEMZERO(pass);
300302
return -1;
301303
}
302-
if (warned && !streq(pass, cp)) {
304+
if (warned && !streq(pass, *p)) {
303305
warned = false;
304306
}
305-
ret = STRTCPY (pass, cp);
306-
erase_pass (cp);
307+
ret = STRTCPY(pass, *p);
308+
p = passzero(p);
307309
if (ret == -1) {
308310
(void) fputs (_("Password is too long.\n"), stderr);
309311
MEMZERO(orig);
@@ -327,17 +329,17 @@ static int new_password (const struct passwd *pw)
327329
warned = true;
328330
continue;
329331
}
330-
cp = agetpass (_("Re-enter new password: "));
331-
if (NULL == cp) {
332+
p = getpass2(p, _("Re-enter new password: "));
333+
if (NULL == p) {
332334
MEMZERO(orig);
333335
MEMZERO(pass);
334336
return -1;
335337
}
336-
if (!streq(cp, pass)) {
337-
erase_pass (cp);
338+
if (!streq(*p, pass)) {
339+
p = passzero(p);
338340
(void) fputs (_("They don't match; try again.\n"), stderr);
339341
} else {
340-
erase_pass (cp);
342+
passzero(p);
341343
break;
342344
}
343345
}
@@ -1086,12 +1088,14 @@ main(int argc, char **argv)
10861088
*/
10871089
if (!anyflag && use_pam) {
10881090
if (sflg) {
1089-
cp = agetpass_stdin ();
1090-
if (cp == NULL) {
1091+
pass_t *p;
1092+
1093+
p = getpassa_stdin();
1094+
if (p == NULL) {
10911095
exit (E_FAILURE);
10921096
}
1093-
do_pam_passwd_non_interactive ("passwd", name, cp);
1094-
erase_pass (cp);
1097+
do_pam_passwd_non_interactive ("passwd", name, *p);
1098+
passzero(p);
10951099
} else {
10961100
do_pam_passwd (name, qflg, kflg);
10971101
}

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@*/
@@ -58,6 +58,7 @@ int
5858
main(int argc, char *argv[])
5959
{
6060
int err = 0;
61+
pass_t *pass;
6162
char **envp = environ;
6263
TERMIO termio;
6364
struct passwd pwent = {};
@@ -128,8 +129,8 @@ main(int argc, char *argv[])
128129
(void) signal (SIGALRM, catch_signals); /* exit if the timer expires */
129130
(void) alarm (ALARM); /* only wait so long ... */
130131

132+
pass = passalloca();
131133
do { /* repeatedly get login/password pairs */
132-
char *pass;
133134
const char *prompt;
134135

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

152153
/* get a password for root */
153-
pass = agetpass(prompt);
154+
pass = getpass2(pass, prompt);
154155

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

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

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

0 commit comments

Comments
 (0)