Re: a bug in read

[Pavel Roskin]

On Tue, 2008-07-08 at 00:18 +0200, Yoshinori K. Okuji wrote:
> Hello,
> 
> I have noticed that read.c has a bug. In this line:
> 
>   while ((line[i - 1] != '\n') && (line[i - 1] != '\r'))
> 
> LINE is not initialized yet at the first time, so this refers to a 
> uninitialized location.

Thank you!  What's worse, i is 0, so we are reading outside the buffer.
I think this patch should do what the code was meant to do:

diff --git a/commands/read.c b/commands/read.c
index 1995918..96519f8 100644
--- a/commands/read.c
+++ b/commands/read.c
@@ -30,15 +30,16 @@ grub_getline (void)
   int i;
   char *line;
   char *tmp;
+  char last = 0;
 
   i = 0;
   line = grub_malloc (1 + i + sizeof('\0'));
   if (! line)
     return NULL;
 
-  while ((line[i - 1] != '\n') && (line[i - 1] != '\r'))
+  while ((last != '\n') && (last != '\r'))
     {
-      line[i] = grub_getkey ();
+      last = line[i] = grub_getkey ();
       if (grub_isprint (line[i]))
        grub_putchar (line[i]);
       i++;


We should test all grub utilities in Valgrind to find such problems.

By the way, read is not a part of grub-emu.  We'll need to improve the
build system to make such oversights less likely.  We also need "exit"
in grub-emu, as "reboot" doesn't sound right.

-- 
Regards,
Pavel Roskin