getpass fgets check
William Pitcock
nenolod at dereferenced.org
Tue Dec 20 12:40:28 UTC 2011
Hi,
On Tue, Dec 20, 2011 at 2:43 AM, Joakim Tjernlund
<joakim.tjernlund at transmode.se> wrote:
>> From: Daniel Wainwright <wainwright.daniel at gmail.com>
>> To: uclibc at uclibc.org
>> Date: 2011/12/20 08:44
>> Subject: getpass fgets check
>> Sent by: uclibc-bounces at uclibc.org
>>
>> Hi,
>>
>> I believe there is a simple error in getpass.c, line 80:
>>
>>
>>
>> static char buf[PWD_BUFFER_SIZE];
>>
>> ...
>>
>> /* Read the password. */
>> fgets (buf, PWD_BUFFER_SIZE-1, in);
>> if (buf != NULL)
>>
>> ...
>>
>>
>>
>> So the result of fgets is not being checked here, results in reading the
>> buffer uninitialised below.
>
> yes, and I think(if max passwd len is important) that it should read
> fgets (buf, PWD_BUFFER_SIZE, in)
> as fgets man page says:
> fgets() reads in at most one less than size characters from stream and
> stores them into the buffer pointed to by s.
I think that using 'sizeof buf' is cleaner and more futureproof than
depending on PWD_BUFFER_SIZE. Something like this should cleanly fix
the potential usage of buf whilst in uninitialized state:
diff --git a/libc/unistd/getpass.c b/libc/unistd/getpass.c
index 8d80182..b8cb640 100644
--- a/libc/unistd/getpass.c
+++ b/libc/unistd/getpass.c
@@ -76,9 +76,11 @@ char * getpass (const char *prompt)
fputs(prompt, out);
fflush(out);
- /* Read the password. */
- fgets (buf, PWD_BUFFER_SIZE-1, in);
- if (buf != NULL)
+ /* Read the password, ensuring buf is initialized first as fgets() may not
+ touch the buffer itself. */
+ *buf = '\0';
+ fgets (buf, sizeof buf, in);
+ if (*buf != '\0')
{
nread = strlen(buf);
if (nread < 0)
Please test and see if that fixes your problem.
William
More information about the uClibc
mailing list