Re: [Nano-devel] Crash report - segfault reproduceable

[David Lawrence Ramsey]

Mike Frysinger wrote:

<snip>

> indeed ... constantly resizing and pounding the arrow keys yields no
> crash
>
> kudos !

Thank you.  I'm attaching patches to add the same fix to 1.3.7 through
1.3.10, since this is a longstanding problem.

diff -ur nano-1.3.7/src/winio.c nano-1.3.7-fixed/src/winio.c
--- nano-1.3.7/src/winio.c      2005-04-10 23:51:22.000000000 -0400
+++ nano-1.3.7-fixed/src/winio.c        2006-02-02 23:47:01.000000000 -0500
@@ -2253,11 +2253,22 @@
 
     assert(column <= start_col);
 
-    /* Allocate enough space for the entire line.  It should contain
-     * (len + 2) multibyte characters at most. */
-    alloc_len = mb_cur_max() * (len + 2);
+    /* Make sure there's enough room for the initial character, whether
+     * it's a multibyte control character, a non-control multibyte
+     * character, a tab character, or a null terminator.  Rationale:
+     *
+     * multibyte control character followed by a null terminator:
+     *     1 byte ('^') + mb_cur_max() bytes + 1 byte ('\0')
+     * multibyte non-control character followed by a null terminator:
+     *     mb_cur_max() bytes + 1 byte ('\0')
+     * tab character followed by a null terminator:
+     *     mb_cur_max() bytes + (tabsize - 1) bytes + 1 byte ('\0')
+     *
+     * Since tabsize has a minimum value of 1, it can substitute for 1
+     * byte above. */
+    alloc_len = (mb_cur_max() + tabsize + 1) * 128;
+    converted = charalloc(alloc_len);
 
-    converted = charalloc(alloc_len + 1);
     index = 0;
 
     if (column < start_col || (dollars && column > 0 &&
@@ -2295,10 +2306,18 @@
 #endif
     }
 
-    while (index < alloc_len - 1 && buf[start_index] != '\0') {
+    while (buf[start_index] != '\0') {
        buf_mb_len = parse_mbchar(buf + start_index, buf_mb, &bad_char,
                NULL);
 
+       /* Make sure there's enough room for the next character, whether
+        * it's a multibyte control character, a non-control multibyte
+        * character, a tab character, or a null terminator. */
+       if (index + mb_cur_max() + tabsize + 1 >= alloc_len - 1) {
+           alloc_len += (mb_cur_max() + tabsize + 1) * 128;
+           converted = charealloc(converted, alloc_len);
+       }
+
        if (*buf_mb == '\t') {
 #if !defined(NANO_SMALL) && defined(ENABLE_NANORC)
            if (ISSET(WHITESPACE_DISPLAY)) {
@@ -2379,8 +2398,10 @@
        start_index += buf_mb_len;
     }
 
-    if (index < alloc_len - 1)
-       converted[index] = '\0';
+    assert(alloc_len >= index + 1);
+
+    /* Null terminate converted. */
+    converted[index] = '\0';
 
     /* Make sure converted takes up no more than len columns. */
     index = actual_x(converted, len);

diff -ur nano-1.3.8/src/winio.c nano-1.3.8-fixed/src/winio.c
--- nano-1.3.8/src/winio.c      2005-06-30 12:18:35.000000000 -0400
+++ nano-1.3.8-fixed/src/winio.c        2006-02-02 23:42:55.000000000 -0500
@@ -2283,11 +2283,22 @@
 
     assert(column <= start_col);
 
-    /* Allocate enough space for the entire line, accounting for a
-     * trailing multibyte character and/or tab. */
-    alloc_len = (mb_cur_max() * (len + 1)) + tabsize;
+    /* Make sure there's enough room for the initial character, whether
+     * it's a multibyte control character, a non-control multibyte
+     * character, a tab character, or a null terminator.  Rationale:
+     *
+     * multibyte control character followed by a null terminator:
+     *     1 byte ('^') + mb_cur_max() bytes + 1 byte ('\0')
+     * multibyte non-control character followed by a null terminator:
+     *     mb_cur_max() bytes + 1 byte ('\0')
+     * tab character followed by a null terminator:
+     *     mb_cur_max() bytes + (tabsize - 1) bytes + 1 byte ('\0')
+     *
+     * Since tabsize has a minimum value of 1, it can substitute for 1
+     * byte above. */
+    alloc_len = (mb_cur_max() + tabsize + 1) * MAX_BUF_SIZE;
+    converted = charalloc(alloc_len);
 
-    converted = charalloc(alloc_len + 1);
     index = 0;
 
     if (column < start_col || (dollars && column > 0 &&
@@ -2325,10 +2336,18 @@
 #endif
     }
 
-    while (index < alloc_len - 1 && buf[start_index] != '\0') {
+    while (buf[start_index] != '\0') {
        buf_mb_len = parse_mbchar(buf + start_index, buf_mb, &bad_char,
                NULL);
 
+       /* Make sure there's enough room for the next character, whether
+        * it's a multibyte control character, a non-control multibyte
+        * character, a tab character, or a null terminator. */
+       if (index + mb_cur_max() + tabsize + 1 >= alloc_len - 1) {
+           alloc_len += (mb_cur_max() + tabsize + 1) * MAX_BUF_SIZE;
+           converted = charealloc(converted, alloc_len);
+       }
+
        if (*buf_mb == '\t') {
 #if !defined(NANO_SMALL) && defined(ENABLE_NANORC)
            if (ISSET(WHITESPACE_DISPLAY)) {
@@ -2411,8 +2430,10 @@
 
     free(buf_mb);
 
-    if (index < alloc_len - 1)
-       converted[index] = '\0';
+    assert(alloc_len >= index + 1);
+
+    /* Null terminate converted. */
+    converted[index] = '\0';
 
     /* Make sure converted takes up no more than len columns. */
     index = actual_x(converted, len);

diff -ur nano-1.3.9/src/winio.c nano-1.3.9-fixed/src/winio.c
--- nano-1.3.9/src/winio.c      2005-10-06 01:28:37.000000000 -0400
+++ nano-1.3.9-fixed/src/winio.c        2006-02-02 23:37:17.000000000 -0500
@@ -2329,10 +2329,22 @@
 
     assert(column <= start_col);
 
-    /* Allocate enough space for the entire line. */
-    alloc_len = (mb_cur_max() * (COLS + 1));
+    /* Make sure there's enough room for the initial character, whether
+     * it's a multibyte control character, a non-control multibyte
+     * character, a tab character, or a null terminator.  Rationale:
+     *
+     * multibyte control character followed by a null terminator:
+     *     1 byte ('^') + mb_cur_max() bytes + 1 byte ('\0')
+     * multibyte non-control character followed by a null terminator:
+     *     mb_cur_max() bytes + 1 byte ('\0')
+     * tab character followed by a null terminator:
+     *     mb_cur_max() bytes + (tabsize - 1) bytes + 1 byte ('\0')
+     *
+     * Since tabsize has a minimum value of 1, it can substitute for 1
+     * byte above. */
+    alloc_len = (mb_cur_max() + tabsize + 1) * MAX_BUF_SIZE;
+    converted = charalloc(alloc_len);
 
-    converted = charalloc(alloc_len + 1);
     index = 0;
 
     if (buf[start_index] != '\t' && (column < start_col || (dollars &&
@@ -2369,9 +2381,17 @@
 #endif
     }
 
-    while (index < alloc_len - 1 && buf[start_index] != '\0') {
+    while (buf[start_index] != '\0') {
        buf_mb_len = parse_mbchar(buf + start_index, buf_mb, NULL);
 
+       /* Make sure there's enough room for the next character, whether
+        * it's a multibyte control character, a non-control multibyte
+        * character, a tab character, or a null terminator. */
+       if (index + mb_cur_max() + tabsize + 1 >= alloc_len - 1) {
+           alloc_len += (mb_cur_max() + tabsize + 1) * MAX_BUF_SIZE;
+           converted = charealloc(converted, alloc_len);
+       }
+
        /* If buf contains a tab character, interpret it. */
        if (*buf_mb == '\t') {
 #if !defined(NANO_SMALL) && defined(ENABLE_NANORC)
@@ -2443,8 +2463,10 @@
 
     free(buf_mb);
 
-    if (index < alloc_len - 1)
-       converted[index] = '\0';
+    assert(alloc_len >= index + 1);
+
+    /* Null terminate converted. */
+    converted[index] = '\0';
 
     /* Make sure converted takes up no more than len columns. */
     index = actual_x(converted, len);

diff -ur nano-1.3.10/src/winio.c nano-1.3.10-fixed/src/winio.c
--- nano-1.3.10/src/winio.c     2005-12-08 02:24:54.000000000 -0500
+++ nano-1.3.10-fixed/src/winio.c       2006-02-02 23:26:52.000000000 -0500
@@ -1770,10 +1770,22 @@
 
     assert(column <= start_col);
 
-    /* Allocate enough space for the entire line. */
-    alloc_len = (mb_cur_max() * (COLS + 1));
+    /* Make sure there's enough room for the initial character, whether
+     * it's a multibyte control character, a non-control multibyte
+     * character, a tab character, or a null terminator.  Rationale:
+     *
+     * multibyte control character followed by a null terminator:
+     *     1 byte ('^') + mb_cur_max() bytes + 1 byte ('\0')
+     * multibyte non-control character followed by a null terminator:
+     *     mb_cur_max() bytes + 1 byte ('\0')
+     * tab character followed by a null terminator:
+     *     mb_cur_max() bytes + (tabsize - 1) bytes + 1 byte ('\0')
+     *
+     * Since tabsize has a minimum value of 1, it can substitute for 1
+     * byte above. */
+    alloc_len = (mb_cur_max() + tabsize + 1) * MAX_BUF_SIZE;
+    converted = charalloc(alloc_len);
 
-    converted = charalloc(alloc_len + 1);
     index = 0;
 
     if (buf[start_index] != '\t' && (column < start_col || (dollars &&
@@ -1810,9 +1822,17 @@
 #endif
     }
 
-    while (index < alloc_len - 1 && buf[start_index] != '\0') {
+    while (buf[start_index] != '\0') {
        buf_mb_len = parse_mbchar(buf + start_index, buf_mb, NULL);
 
+       /* Make sure there's enough room for the next character, whether
+        * it's a multibyte control character, a non-control multibyte
+        * character, a tab character, or a null terminator. */
+       if (index + mb_cur_max() + tabsize + 1 >= alloc_len - 1) {
+           alloc_len += (mb_cur_max() + tabsize + 1) * MAX_BUF_SIZE;
+           converted = charealloc(converted, alloc_len);
+       }
+
        /* If buf contains a tab character, interpret it. */
        if (*buf_mb == '\t') {
 #if !defined(NANO_TINY) && defined(ENABLE_NANORC)
@@ -1884,8 +1904,10 @@
 
     free(buf_mb);
 
-    if (index < alloc_len - 1)
-       converted[index] = '\0';
+    assert(alloc_len >= index + 1);
+
+    /* Null terminate converted. */
+    converted[index] = '\0';
 
     /* Make sure converted takes up no more than len columns. */
     index = actual_x(converted, len);