Skip to content

Commit

Permalink
Add scroll/sensor loading allocation bounds and fix leaks.
Browse files Browse the repository at this point in the history
  • Loading branch information
AliceLR committed May 7, 2024
1 parent 43aa41a commit 5a3f6db
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
1 change: 1 addition & 0 deletions docs/changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ USERS
characters past the first 64 will clip.
+ Clicking a line in the scroll display and editor now jumps to
that line (same as the help file).
+ Fix possible robot stack and scroll message memory leaks.

DEVELOPERS

Expand Down
31 changes: 29 additions & 2 deletions src/robot.c
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ static int load_robot_from_memory(struct world *mzx_world, struct robot *cur_rob
// # of stack values is file size /4.
// Furthermore, there should be an even number of values, and a count
// over ROBOT_MAX_STACK is invalid.
if(cur_robot->stack)
break;
size = MIN((size / 4) & ~1, ROBOT_MAX_STACK);
cur_robot->stack_size = size;
cur_robot->stack = cmalloc(size * sizeof(int));
Expand Down Expand Up @@ -500,7 +502,15 @@ struct scroll *load_scroll_allocate(struct zip_archive *zp)
int size;

zip_get_next_uncompressed_size(zp, &actual_size);
if(actual_size > SCROLL_PROPS_SIZE + MAX_OBJ_SIZE)
actual_size = SCROLL_PROPS_SIZE + MAX_OBJ_SIZE;

buffer = cmalloc(actual_size);
if(!buffer)
{
zip_skip_file(zp);
goto err;
}

// We aren't saving or loading null scrolls.
cur_scroll->used = 1;
Expand All @@ -522,8 +532,15 @@ struct scroll *load_scroll_allocate(struct zip_archive *zp)
break;

case SCRPROP_MESG:
if(cur_scroll->mesg)
break;
if(size < 3)
goto err;

// TODO: return value (scroll display doesn't support NULL here)
size = MIN(size, MAX_OBJ_SIZE);
cur_scroll->mesg_size = size;
cur_scroll->mesg = cmalloc(size);
cur_scroll->mesg = (char *)cmalloc(size);
mfread(cur_scroll->mesg, size, 1, &prop);
if(size > 0)
cur_scroll->mesg[size - 1] = '\0';
Expand All @@ -536,12 +553,14 @@ struct scroll *load_scroll_allocate(struct zip_archive *zp)

if(cur_scroll->mesg_size < 3)
{
err:
// We have an incomplete scroll, so slip in an empty scroll.
cur_scroll->num_lines = 1;
cur_scroll->mesg_size = 3;

// TODO: return value (scroll display doesn't support NULL here)
free(cur_scroll->mesg);
cur_scroll->mesg = cmalloc(3);
cur_scroll->mesg = (char *)cmalloc(3);
strcpy(cur_scroll->mesg, "\x01\x0A");
}

Expand All @@ -561,7 +580,15 @@ struct sensor *load_sensor_allocate(struct zip_archive *zp)
int size;

zip_get_next_uncompressed_size(zp, &actual_size);
if(actual_size > MAX_OBJ_SIZE) // SENSOR_PROPS_SIZE, but just in case...
actual_size = MAX_OBJ_SIZE;

buffer = cmalloc(actual_size);
if(!buffer)
{
zip_skip_file(zp);
return cur_sensor;
}

// We aren't saving or loading null sensors.
cur_sensor->used = 1;
Expand Down

0 comments on commit 5a3f6db

Please sign in to comment.