Ctrl-k in an edit screen does not work correctly

[Hideki EIRAKU]

Hello,

This is a bug report with a patch.
Ctrl-k in an edit screen does not work correctly.
Steps to reproduce:
1. Start GNU GRUB2 with some menu entries.
2. Type 'e' to edit the menu.
3. Type ctrl-k to cut the first line. (Note: the first line should
   have one or more characters.)
4. The system suddenly starts rebooting!
   (If you runs GRUB2 in QEMU, QEMU may stop with abort messages.)

I tried version 1.99. I found the bug in the source code, then I
looked at the latest bzr source code and the bug seems still there.

The system rebooting is caused by a simple buffer overflow in the
function kill_line() in grub-core/normal/menu_entry.c:

------------------------------------------------------------
      for (i = 0; i < screen->nterms; i++)
        orig_num[i] = get_logical_num_lines (linep, &screen->terms[i]);
      linep->len = screen->column;

      if (update)
        {
          new_num = get_logical_num_lines (linep, &screen->terms[i]);
------------------------------------------------------------

The last &screen->terms[i] refers to out of the array, because the
variable i is equals to screen->nterms at the line, and the
screen->terms array is allocated as below by a function in the same
source file:

------------------------------------------------------------
  screen->terms = grub_malloc (screen->nterms * sizeof (screen->terms[0]));
------------------------------------------------------------

The kill_line() function has another bug that removes the last
character in the killed_text buffer. It seems that no one has tested
this function yet.

I made a patch that fixes these ctrl-k bugs:
------------------------------------------------------------
=== modified file 'grub-core/normal/menu_entry.c'
--- grub-core/normal/menu_entry.c       2011-11-30 15:20:13 +0000
+++ grub-core/normal/menu_entry.c       2012-02-28 15:51:45 +0000
@@ -890,7 +890,7 @@
        return 0;
 
       grub_memmove (p + offset, linep->buf + screen->column, size);
-      p[offset + size - 1] = '\0';
+      p[offset + size] = '\0';
 
       screen->killed_text = p;
 
@@ -900,9 +900,9 @@
 
       if (update)
        {
-         new_num = get_logical_num_lines (linep, &screen->terms[i]);
          for (i = 0; i < screen->nterms; i++)
            {
+             new_num = get_logical_num_lines (linep, &screen->terms[i]);
              if (orig_num[i] != new_num)
                update_screen (screen, &screen->terms[i],
                               screen->line, screen->column, 0, 1, ALL_LINES);

------------------------------------------------------------

Thanks,
-- 
Hideki EIRAKU <address@hidden>