emo/shadow
1
0
Fork 0
Code Issues Pull Requests Packages Releases Activity

libmisc: agetpass(): Fix bug detecting truncation

Browse Source 
On 2/19/23 18:09, David Mudrich wrote:
> I am working on a RAM based Linux OS from source, and try to use
> latest versions of all software.  I found shadow needs libbsd's
> readpassphrase(3) as superior alternative to getpass(3).  While
> considering if I a) include libbsd, or include libbsd's code of
> readpassphrase(3) into shadow, found, that libbsd's readpassphrase(3)
> never returns \n or \r
> <https://cgit.freedesktop.org/libbsd/tree/src/readpassphrase.c>
> line 122, while agetpass() uses a check for \n in agetpass.c line 108.
> I assume it always fails.

Indeed, it always failed.  I made a mistake when writing agetpass(),
assuming that readpassphrase(3) would keep newlines.

>
> I propose a check of len == PASS_MAX - 1, with false positive error for
> exactly PASS_MAX - 1 long passwords.

Instead, I added an extra byte to the allocation to allow a maximum
password length of PASS_MAX (which is the maximum for getpass(3), which
we're replacing.

While doing that, I notice that my previous implementation also had
another bug (minor): The maximum password length was PASS_MAX - 1
instead of PASS_MAX.  That's also fixed in this commit.

Reported-by: David Mudrich <dmudrich@gmx.de>
Fixes: 155c9421b9 ("libmisc: agetpass(), erase_pass(): Add functions for getting passwords safely")
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This commit is contained in:
Alejandro Colomar 2023-02-19 19:26:56 +01:00 committed by Iker Pedrosa
parent baae5b4a06
commit 5c5dc75641
1 changed files with 14 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
libmisc/agetpass.c
View File

@ -19,7 +19,7 @@
#if !defined(PASS_MAX) #if !defined(PASS_MAX)
#define PASS_MAX BUFSIZ #define PASS_MAX BUFSIZ - 1
#endif #endif
@ -93,29 +93,31 @@ agetpass(const char *prompt)
char *pass; char *pass;
size_t len; size_t len;
pass = malloc(PASS_MAX); /*
* Since we want to support passwords upto PASS_MAX, we need
* PASS_MAX bytes for the password itself, and one more byte for
* the terminating '\0'. We also want to detect truncation, and
* readpassphrase(3) doesn't detect it, so we need some trick.
* Let's add one more byte, and if the password uses it, it
* means the introduced password was longer than PASS_MAX.
*/
pass = malloc(PASS_MAX + 2);
if (pass == NULL) if (pass == NULL)
return NULL; return NULL;
if (readpassphrase(prompt, pass, PASS_MAX, RPP_REQUIRE_TTY) == NULL) if (readpassphrase(prompt, pass, PASS_MAX + 2, RPP_REQUIRE_TTY) == NULL)
goto fail; goto fail;
len = strlen(pass); len = strlen(pass);
if (len == PASS_MAX + 1) {
if (len == 0)
return pass;
if (pass[len - 1] != '\n') {
errno = ENOBUFS; errno = ENOBUFS;
goto fail; goto fail;
} }
pass[len - 1] = '\0';
return pass; return pass;
fail: fail:
freezero(pass, PASS_MAX); freezero(pass, PASS_MAX + 2);
return NULL; return NULL;
} }
@ -123,5 +125,5 @@ fail:
void void
erase_pass(char *pass) erase_pass(char *pass)
{ {
freezero(pass, PASS_MAX); freezero(pass, PASS_MAX + 2);
} }