BB vi bug

[Denys Vlasenko]

On Friday 20 June 2008 17:42, walter harms wrote:
> Hello Denys,
> can you please look at "vi.c coredump" mail ?
> 
> I found that that stupid_insert() can return NULL (what is never checked)
> and therefore can crash vi. it is not a security issue but very annoying.
> unfortunately is that a core function and i feel not good to add simple
> checks without understand what they may cause.

You are assuming that I know busybox vi.c :)
I don't. Nobody knows it :)

Okay.

static char *stupid_insert(char * p, char c) // stupidly insert the char c at 'p'
{
        p = text_hole_make(p, 1);
        if (p != 0) {
                *p = c;
                file_modified++;        // has the file been modified
                p++;
        }
        return p;
}

Neat function, yes. it will silently fail to insert char if p == NULL.
How nice. I bet callers don't expect that!

//  open a hole in text[]
static char *text_hole_make(char * p, int size) // at "p", make a 'size' byte hole
{
        char *src, *dest;
        int cnt;

        if (size <= 0)
                goto thm0;
        src = p;
        dest = p + size;
        cnt = end - src;        // the rest of buffer
        if ( ((end + size) >= (text + text_size)) // TODO: realloc here
                        || memmove(dest, src, cnt) != dest) {
                status_line_bold("can't create room for new characters");
                p = NULL;
                goto thm0;
        }
        memset(p, ' ', size);   // clear new hole
        end += size;            // adjust the new END
        file_modified++;        // has the file been modified
 thm0:
        return p;
}

More coolness. "if memmove(dest, src, cnt) != dest ..." - what?!
Since when memmove can return something else than dest?

Wait. Look closely. src, dest, cnt are all NEVER USED!  8]

Superfluous file_modified++ in stupid_insert()...

Oh well.

Try this patch please.
--
vda
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 9.patch
Type: text/x-diff
Size: 4504 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20080620/4c86efd1/attachment.bin