[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Ctrl-k in an edit screen does not work correctly

From: Hideki EIRAKU
Subject: Ctrl-k in an edit screen does not work correctly
Date: Wed, 29 Feb 2012 01:17:51 +0900 (JST)


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);


Hideki EIRAKU <address@hidden>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]