From 67e82f1a0bc2a9078cfe11e0add190fa7cc4b891 Mon Sep 17 00:00:00 2001 From: Helmut Grohne Date: Tue, 19 Jan 2010 20:24:19 +0100 Subject: fixed a few buffer overruns strncpy does not always terminate strings. Unterminated strings (without a length) are bad. So better terminate them. --- src/map/chrif.c | 9 ++++++--- src/map/clif.c | 4 ++-- src/map/npc.c | 8 +++++--- src/map/pc.c | 3 ++- src/map/script.c | 7 ++++--- 5 files changed, 19 insertions(+), 12 deletions(-) (limited to 'src/map') diff --git a/src/map/chrif.c b/src/map/chrif.c index c4a528b..1f5673a 100644 --- a/src/map/chrif.c +++ b/src/map/chrif.c @@ -51,7 +51,8 @@ static int chrif_state; */ void chrif_setuserid (char *id) { - strncpy (userid, id, 24); + strncpy (userid, id, sizeof(userid)-1); + userid[sizeof(userid)-1] = '\0'; } /*========================================== @@ -60,7 +61,8 @@ void chrif_setuserid (char *id) */ void chrif_setpasswd (char *pwd) { - strncpy (passwd, pwd, 24); + strncpy (passwd, pwd, sizeof(passwd)-1); + passwd[sizeof(passwd)-1] = '\0'; } /*========================================== @@ -69,7 +71,8 @@ void chrif_setpasswd (char *pwd) */ void chrif_setip (char *ip) { - strncpy (char_ip_str, ip, 16); + strncpy (char_ip_str, ip, sizeof(char_ip_str)-1); + char_ip_str[sizeof(char_ip_str)-1] = '\0'; char_ip = inet_addr (char_ip_str); } diff --git a/src/map/clif.c b/src/map/clif.c index c3099d7..86be79c 100644 --- a/src/map/clif.c +++ b/src/map/clif.c @@ -8204,14 +8204,14 @@ void clif_parse_NpcStringInput (int fd, struct map_session_data *sd) len = RFIFOW (fd, 2) - 7; - if (len >= sizeof (sd->npc_str)) + if (len >= sizeof (sd->npc_str)-1) { printf ("clif: input string too long !\n"); memcpy (sd->npc_str, RFIFOP (fd, 8), sizeof (sd->npc_str)); - sd->npc_str[sizeof (sd->npc_str) - 1] = 0; } else strncpy (sd->npc_str, RFIFOP (fd, 8), len); + sd->npc_str[sizeof (sd->npc_str) - 1] = 0; map_scriptcont (sd, RFIFOL (fd, 4)); } diff --git a/src/map/npc.c b/src/map/npc.c index 49fe578..4ff5ba2 100644 --- a/src/map/npc.c +++ b/src/map/npc.c @@ -321,7 +321,8 @@ int npc_event_doall_l (const char *name, int rid, int argc, argrec_t * args) int c = 0; char buf[64] = "::"; - strncpy (buf + 2, name, 62); + strncpy (buf + 2, name, sizeof(buf)-3); + buf[sizeof(buf)-1] = '\0'; strdb_foreach (ev_db, npc_event_doall_sub, &c, buf, rid, argc, args); return c; } @@ -1477,7 +1478,8 @@ int npc_convertlabel_db (void *key, void *data, va_list ap) * (num + 1)); *p = '\0'; - strncpy (lst[num].name, lname, 24); + strncpy (lst[num].name, lname, sizeof(lst[num].name)-1); + lst[num].name[sizeof(lst[num].name)-1] = '\0'; *p = ':'; lst[num].pos = pos; nd->u.scr.label_list = lst; @@ -1856,7 +1858,7 @@ static int npc_parse_function (char *w1, char *w2, char *w3, char *w4, p = (char *) aCalloc (50, sizeof (char)); - strncpy (p, w3, 50); + strncpy (p, w3, 49); strdb_insert (script_get_userfunc_db (), p, script); // label_db=script_get_label_db(); diff --git a/src/map/pc.c b/src/map/pc.c index 689bcd2..9741852 100644 --- a/src/map/pc.c +++ b/src/map/pc.c @@ -8359,7 +8359,8 @@ int pc_setsavepoint (struct map_session_data *sd, char *mapname, int x, int y) { nullpo_retr (0, sd); - strncpy (sd->status.save_point.map, mapname, 24); + strncpy (sd->status.save_point.map, mapname, 23); + sd->status.save_point.map[23] = '\0'; sd->status.save_point.x = x; sd->status.save_point.y = y; diff --git a/src/map/script.c b/src/map/script.c index bbde20c..03a092e 100644 --- a/src/map/script.c +++ b/src/map/script.c @@ -5691,7 +5691,7 @@ int buildin_getcastlename (struct script_state *st) if (strcmp (mapname, gc->map_name) == 0) { buf = (char *) aCalloc (24, sizeof (char)); - strncpy (buf, gc->castle_name, 24); + strncpy (buf, gc->castle_name, 23); break; } } @@ -6942,10 +6942,10 @@ int buildin_getsavepoint (struct script_state *st) x = sd->status.save_point.x; y = sd->status.save_point.y; - strncpy (mapname, sd->status.save_point.map, 24); switch (type) { case 0: + strncpy (mapname, sd->status.save_point.map, 23); push_str (st->stack, C_STR, mapname); break; case 1: @@ -7068,7 +7068,8 @@ int buildin_fakenpcname (struct script_state *st) nd = npc_name2id (name); if (!nd) return 1; - strncpy (nd->name, newname, 24); + strncpy (nd->name, newname, sizeof(nd->name)-1); + nd->name[sizeof(nd->name)-1] = '\0'; nd->class = newsprite; // Refresh this npc -- cgit v1.2.3-70-g09d2 From ea5866863cbdad80eb69351417018c6104c5f43b Mon Sep 17 00:00:00 2001 From: Helmut Grohne Date: Tue, 19 Jan 2010 20:33:25 +0100 Subject: fixed a few memory leaks --- src/map/magic-interpreter-parser.y | 2 ++ src/map/magic-stmt.c | 4 +++- src/map/script.c | 4 +--- 3 files changed, 6 insertions(+), 4 deletions(-) (limited to 'src/map') diff --git a/src/map/magic-interpreter-parser.y b/src/map/magic-interpreter-parser.y index 047e38c..04d5630 100644 --- a/src/map/magic-interpreter-parser.y +++ b/src/map/magic-interpreter-parser.y @@ -268,6 +268,8 @@ spellconf_option : ID '=' expr if (!failed_flag) add_teleport_anchor(anchor, @1.first_line); + else + free(anchor); failed_flag = 0; } | PROCEDURE ID '(' proc_formals_list ')' '=' effect_list diff --git a/src/map/magic-stmt.c b/src/map/magic-stmt.c index 1a1ced7..93ef65f 100644 --- a/src/map/magic-stmt.c +++ b/src/map/magic-stmt.c @@ -1161,7 +1161,7 @@ static effect_t *run_foreach (invocation_t * invocation, effect_t * foreach, cont_activation_record_t *ar = add_stack_entry (invocation, CONT_STACK_FOREACH, return_location); int entities_allocd = 64; - int *entities_collect = malloc (entities_allocd * sizeof (int)); + int *entities_collect; int *entities; int *shuffle_board; int entities_nr = 0; @@ -1170,6 +1170,8 @@ static effect_t *run_foreach (invocation_t * invocation, effect_t * foreach, if (!ar) return return_location; + entities_collect = malloc (entities_allocd * sizeof (int)); + find_entities_in_area (area.v.v_area, &entities_allocd, &entities_nr, &entities_collect, filter); diff --git a/src/map/script.c b/src/map/script.c index 03a092e..0ede96c 100644 --- a/src/map/script.c +++ b/src/map/script.c @@ -6264,7 +6264,6 @@ int buildin_strmobinfo (struct script_state *st) if (num == 1) { char *buf; - buf = calloc (24, 1); buf = mob_db[class].name; push_str (st->stack, C_STR, buf); return 0; @@ -6272,7 +6271,6 @@ int buildin_strmobinfo (struct script_state *st) else if (num == 2) { char *buf; - buf = calloc (24, 1); buf = mob_db[class].jname; push_str (st->stack, C_STR, buf); return 0; @@ -6938,13 +6936,13 @@ int buildin_getsavepoint (struct script_state *st) sd = script_rid2sd (st); type = conv_num (st, &(st->stack->stack_data[st->start + 2])); - mapname = calloc (24, 1); x = sd->status.save_point.x; y = sd->status.save_point.y; switch (type) { case 0: + mapname = calloc (24, 1); strncpy (mapname, sd->status.save_point.map, 23); push_str (st->stack, C_STR, mapname); break; -- cgit v1.2.3-70-g09d2 From dc0c8b2ca0f3867f1b4641092b9f90738ca8c7af Mon Sep 17 00:00:00 2001 From: Helmut Grohne Date: Tue, 19 Jan 2010 20:34:06 +0100 Subject: fixed a buffer overrun and use of uninitialized The target buffer for the memcpy only takes like 24 chars. strcat on an uninitialized buffer is a bad idea. --- src/map/npc.c | 2 +- src/map/tmw.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src/map') diff --git a/src/map/npc.c b/src/map/npc.c index 4ff5ba2..edbf548 100644 --- a/src/map/npc.c +++ b/src/map/npc.c @@ -74,7 +74,7 @@ int npc_enable_sub (struct block_list *bl, va_list ap) if (nd->flag & 1) // �������������� return 1; - memcpy (name, nd->name, 50); + memcpy (name, nd->name, sizeof(nd->name)); if (sd->areanpc_id == nd->bl.id) return 1; sd->areanpc_id = nd->bl.id; diff --git a/src/map/tmw.c b/src/map/tmw.c index 2849983..3c506c5 100644 --- a/src/map/tmw.c +++ b/src/map/tmw.c @@ -146,7 +146,7 @@ void tmw_GmHackMsg (const char *fmt, ...) va_end (ap); char outbuf[512 + 5]; - strcat (outbuf, "[GM] "); + strcpy (outbuf, "[GM] "); strcat (outbuf, buf); intif_wis_message_to_gm (wisp_server_name, -- cgit v1.2.3-70-g09d2