Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Overflow #7

Open
wants to merge 2 commits into from

2 participants

@msantos

Hey Mazen, this is the same set of patches I sent you about a year ago.

I've split them so you can just pull in the fix for the ei_decode_string() usage.

The fix for the stack overflow still uses malloc/free. Alternatively, the patch could check the size of the string and return an error, if the string is too large (no idea what a good size for that would be).

msantos added some commits
@msantos msantos Fix buffer overflow
Increase the buffer size for strings by one byte to accomodate the
trail NULL expected by ei_decode_string:

    The string is copied to p, and enough space must be allocated. The
    returned string is null terminated so you need to add an extra byte
    to the memory requirement.
8c6d81f
@msantos msantos Prevent stack overflow
Large strings can overflow the stack:

    crash() ->
        cecho:start(0,0),
        S = lists:duplicate(8192*1024*2, "x"),
        cecho:mvaddstr(0,0,S).

Fix the problem by putting the data on the heap.
a70554c
@msantos

The code to crash the VM is hidden in the code commit:

-module(crash).
-export([crash/0]).

% erl -noinput -pa ../cecho/ebin/ -eval 'crash:crash()' +A 50
crash() ->
cecho:start(0,0),
S = lists:duplicate(819210242, "x"),
cecho:mvaddstr(0,0,S).

@mazenharake
Owner

Hi Michael,

Thanks for the fixes, I'll have a look as soon as I got some time and test it out a little bit. Out of curiosity, why would you want to print that large of a string? It won't even fit the screen?! From an ncurses perspective it wouldn't make sense... is there any other reason why this should be moved to heap?

@mazenharake
Owner

Oh and sorry if you sent this a year ago and I didn't do anything about it, it was a hectic period at that time as well :)

@msantos
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 28, 2012
  1. @msantos

    Fix buffer overflow

    msantos authored
    Increase the buffer size for strings by one byte to accomodate the
    trail NULL expected by ei_decode_string:
    
        The string is copied to p, and enough space must be allocated. The
        returned string is null terminated so you need to add an extra byte
        to the memory requirement.
  2. @msantos

    Prevent stack overflow

    msantos authored
    Large strings can overflow the stack:
    
        crash() ->
            cecho:start(0,0),
            S = lists:duplicate(8192*1024*2, "x"),
            cecho:mvaddstr(0,0,S).
    
    Fix the problem by putting the data on the heap.
This page is out of date. Refresh to see the latest.
Showing with 36 additions and 8 deletions.
  1. +36 −8 c_src/cecho.c
View
44 c_src/cecho.c
@@ -224,11 +224,18 @@ void do_addch(state *st) {
void do_addstr(state *st) {
int arity;
long strlen;
+ char *str = NULL;
+ int code = 0;
ei_decode_tuple_header(st->args, &(st->index), &arity);
ei_decode_long(st->args, &(st->index), &strlen);
- char str[strlen];
+ if ( (str = calloc(strlen+1, 1)) == NULL) {
+ encode_ok_reply(st, ENOMEM);
+ return;
+ }
ei_decode_string(st->args, &(st->index), str);
- encode_ok_reply(st, addnstr(str, strlen));
+ code = addnstr(str, strlen);
+ free(str);
+ encode_ok_reply(st, code);
}
void do_move(state *st) {
@@ -340,13 +347,20 @@ void do_mvaddch(state *st) {
void do_mvaddstr(state *st) {
int arity;
long strlen, y, x;
+ char *str = NULL;
+ int code = 0;
ei_decode_tuple_header(st->args, &(st->index), &arity);
ei_decode_long(st->args, &(st->index), &y);
ei_decode_long(st->args, &(st->index), &x);
ei_decode_long(st->args, &(st->index), &strlen);
- char str[strlen];
+ if ( (str = calloc(strlen+1, 1)) == NULL) {
+ encode_ok_reply(st, ENOMEM);
+ return;
+ }
ei_decode_string(st->args, &(st->index), str);
- encode_ok_reply(st, mvaddnstr((int)y, (int)x, str, (int)strlen));
+ code = mvaddnstr((int)y, (int)x, str, (int)strlen);
+ free(str);
+ encode_ok_reply(st, code);
}
void do_newwin(state *st) {
@@ -393,12 +407,19 @@ void do_wmove(state *st) {
void do_waddstr(state *st) {
int arity;
long slot, strlen;
+ char *str = NULL;
+ int code = 0;
ei_decode_tuple_header(st->args, &(st->index), &arity);
ei_decode_long(st->args, &(st->index), &slot);
ei_decode_long(st->args, &(st->index), &strlen);
- char str[strlen];
+ if ( (str = calloc(strlen+1, 1)) == NULL) {
+ encode_ok_reply(st, ENOMEM);
+ return;
+ }
ei_decode_string(st->args, &(st->index), str);
- encode_ok_reply(st, waddnstr(st->win[slot], str, strlen));
+ code = waddnstr(st->win[slot], str, strlen);
+ free(str);
+ encode_ok_reply(st, code);
}
void do_waddch(state *st) {
@@ -414,14 +435,21 @@ void do_waddch(state *st) {
void do_mvwaddstr(state *st) {
int arity;
long slot, y, x, strlen;
+ char *str = NULL;
+ int code = 0;
ei_decode_tuple_header(st->args, &(st->index), &arity);
ei_decode_long(st->args, &(st->index), &slot);
ei_decode_long(st->args, &(st->index), &y);
ei_decode_long(st->args, &(st->index), &x);
ei_decode_long(st->args, &(st->index), &strlen);
- char str[strlen];
+ if ( (str = calloc(strlen+1, 1)) == NULL) {
+ encode_ok_reply(st, ENOMEM);
+ return;
+ }
ei_decode_string(st->args, &(st->index), str);
- encode_ok_reply(st, mvwaddnstr(st->win[slot], (int)y, (int)x, str, strlen));
+ code = mvwaddnstr(st->win[slot], (int)y, (int)x, str, strlen);
+ free(str);
+ encode_ok_reply(st, code);
}
void do_mvwaddch(state *st) {
Something went wrong with that request. Please try again.