From 87a47e241ce55f01ddd66475ff36420e5f020030 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 11:23:57 +0100 Subject: [PATCH 01/15] CID 22479 Dereference after null check CID 22478 Dereference after null check CID 22477 Dereference after null check CID 22476 Dereference after null check CID 22475 Dereference after null check --- src/bindings.c | 33 ++++++++++++++++++++++----------- storage | 2 +- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/bindings.c b/src/bindings.c index c2f18d8d5..20a8192e1 100755 --- a/src/bindings.c +++ b/src/bindings.c @@ -1,4 +1,4 @@ -/* +/* +-------------------+ | | Enno Rehling | Eressea PBEM host | Christian Schlittchen @@ -237,11 +237,15 @@ static int tolua_message_unit(lua_State * L) unit *sender = (unit *)tolua_tousertype(L, 1, 0); unit *target = (unit *)tolua_tousertype(L, 2, 0); const char *str = tolua_tostring(L, 3, 0); - if (!target) + if (!target) { tolua_error(L, TOLUA_CAST "target is nil", NULL); - if (!sender) + } + else if (!sender) { tolua_error(L, TOLUA_CAST "sender is nil", NULL); - deliverMail(target->faction, sender->region, sender, str, target); + } + else { + deliverMail(target->faction, sender->region, sender, str, target); + } return 0; } @@ -250,11 +254,15 @@ static int tolua_message_faction(lua_State * L) unit *sender = (unit *)tolua_tousertype(L, 1, 0); faction *target = (faction *)tolua_tousertype(L, 2, 0); const char *str = tolua_tostring(L, 3, 0); - if (!target) + if (!target) { tolua_error(L, TOLUA_CAST "target is nil", NULL); - if (!sender) + } + else if (!sender) { tolua_error(L, TOLUA_CAST "sender is nil", NULL); - deliverMail(target, sender->region, sender, str, NULL); + } + else { + deliverMail(target, sender->region, sender, str, NULL); + } return 0; } @@ -262,10 +270,13 @@ static int tolua_message_region(lua_State * L) { unit *sender = (unit *)tolua_tousertype(L, 1, 0); const char *str = tolua_tostring(L, 2, 0); - if (!sender) + if (!sender) { tolua_error(L, TOLUA_CAST "sender is nil", NULL); - ADDMSG(&sender->region->msgs, msg_message("mail_result", "unit message", - sender, str)); + } + else { + ADDMSG(&sender->region->msgs, msg_message("mail_result", "unit message", + sender, str)); + } return 0; } @@ -1172,7 +1183,7 @@ int eressea_run(lua_State *L, const char *luafile) if (err != 0) { log_lua_error(L); } - else { + else { if (lua_isnumber(L, -1)) { err = (int)lua_tonumber(L, -1); } diff --git a/storage b/storage index 86b967441..1d92cb36d 160000 --- a/storage +++ b/storage @@ -1 +1 @@ -Subproject commit 86b96744157eb08c55998df4c12fa2e073005b49 +Subproject commit 1d92cb36df41c183c378aad17cbbfc0eddbb5c84 From 731dac336341e4df9ddba57e2eeaef6fd3fe8ba3 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 11:45:13 +0100 Subject: [PATCH 02/15] CID 22437 Unchecked return value from library github issue #332 --- src/kernel/save.c | 5 ++++- src/kernel/save.h | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index 45e9902ca..27eef8206 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -553,8 +553,11 @@ int current_turn(void) perror(zText); } else { - fscanf(F, "%d\n", &cturn); + int c = fscanf(F, "%d\n", &cturn); fclose(F); + if (c != 1) { + return -1; + } } return cturn; } diff --git a/src/kernel/save.h b/src/kernel/save.h index 11b53d620..bb8746e00 100644 --- a/src/kernel/save.h +++ b/src/kernel/save.h @@ -51,7 +51,7 @@ extern "C" { extern int data_version; extern int enc_gamedata; - extern int current_turn(void); + int current_turn(void); extern void read_items(struct storage *store, struct item **it); extern void write_items(struct storage *store, struct item *it); From 3fe7b94cc744f2b3848eefc9eb3b3150f51156a7 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 11:46:57 +0100 Subject: [PATCH 03/15] CID 22436 Ignoring number of bytes read --- src/kernel/save.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index 27eef8206..0c7fbd659 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1409,8 +1409,8 @@ int readgame(const char *filename, bool backup) fread(&gdata.version, sizeof(int), 1, F); if (gdata.version >= INTPAK_VERSION) { int stream_version; - fread(&stream_version, sizeof(int), 1, F); - assert(stream_version == STREAM_VERSION || !"unsupported data format"); + size_t sz = fread(&stream_version, sizeof(int), 1, F); + assert((sz==1 && stream_version == STREAM_VERSION) || !"unsupported data format"); } assert(gdata.version >= MIN_VERSION || !"unsupported data format"); assert(gdata.version <= MAX_VERSION || !"unsupported data format"); From 47f678d2a3685520e5e7fe6b5a02ee25b83a233c Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 11:46:57 +0100 Subject: [PATCH 04/15] CID 22436 Ignoring number of bytes read, github issue #333 --- src/kernel/save.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index 27eef8206..0c7fbd659 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1409,8 +1409,8 @@ int readgame(const char *filename, bool backup) fread(&gdata.version, sizeof(int), 1, F); if (gdata.version >= INTPAK_VERSION) { int stream_version; - fread(&stream_version, sizeof(int), 1, F); - assert(stream_version == STREAM_VERSION || !"unsupported data format"); + size_t sz = fread(&stream_version, sizeof(int), 1, F); + assert((sz==1 && stream_version == STREAM_VERSION) || !"unsupported data format"); } assert(gdata.version >= MIN_VERSION || !"unsupported data format"); assert(gdata.version <= MAX_VERSION || !"unsupported data format"); From a02b5f343e1402dfaeee4756e5d6a2620d531a06 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 11:55:04 +0100 Subject: [PATCH 05/15] CID 22488 Dereference after null check --- src/battle.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/battle.c b/src/battle.c index 853c908d7..dfbdb8099 100644 --- a/src/battle.c +++ b/src/battle.c @@ -4258,9 +4258,10 @@ static bool is_enemy(battle *b, unit *u1, unit *u2) { for (es = b->sides; es != b->sides + b->nsides; ++es) { if (!s1 && es->faction == u1->faction) s1 = es; else if (!s2 && es->faction == u2->faction) s2 = es; - if (s1 && s2) break; + if (s1 && s2) { + return enemy(s1, s2); + } } - return enemy(s1, s2); } else { return !help_enter(u1, u2); From 978dd1400f6bebe748c203aa70acbc8bb602cc5a Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 11:57:13 +0100 Subject: [PATCH 06/15] CID 22465 Division or modulo by zero --- src/battle.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/battle.c b/src/battle.c index dfbdb8099..ae74077a6 100644 --- a/src/battle.c +++ b/src/battle.c @@ -300,9 +300,8 @@ fighter *select_corpse(battle * b, fighter * af) * * Untote werden nicht ausgewählt (casualties, not dead) */ { - int si, di, maxcasualties = 0; + int si, maxcasualties = 0; fighter *df; - side *s; for (si = 0; si != b->nsides; ++si) { side *s = b->sides + si; @@ -310,24 +309,26 @@ fighter *select_corpse(battle * b, fighter * af) maxcasualties += s->casualties; } } - di = (int)(rng_int() % maxcasualties); - for (s = b->sides; s != b->sides + b->nsides; ++s) { - for (df = s->fighters; df; df = df->next) { - /* Geflohene haben auch 0 hp, dürfen hier aber nicht ausgewählt - * werden! */ - int dead = dead_fighters(df); - if (!playerrace(u_race(df->unit))) - continue; + if (maxcasualties > 0) { + int di = (int)(rng_int() % maxcasualties); + side *s; + for (s = b->sides; s != b->sides + b->nsides; ++s) { + for (df = s->fighters; df; df = df->next) { + /* Geflohene haben auch 0 hp, dürfen hier aber nicht ausgewählt + * werden! */ + int dead = dead_fighters(df); + if (!playerrace(u_race(df->unit))) + continue; - if (af && !helping(af->side, df->side)) - continue; - if (di < dead) { - return df; + if (af && !helping(af->side, df->side)) + continue; + if (di < dead) { + return df; + } + di -= dead; } - di -= dead; } } - return NULL; } From d6cd1feb489095851fd6a258b2c1e19d3d84ce10 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 11:59:17 +0100 Subject: [PATCH 07/15] CID 22456: 'Constant' variable guards dead code github issue #360 prayers sind ein feature, das nie fertig entwickelt wurde --- src/battle.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/battle.c b/src/battle.c index ae74077a6..2e99c60b1 100644 --- a/src/battle.c +++ b/src/battle.c @@ -3247,7 +3247,6 @@ fighter *make_fighter(battle * b, unit * u, side * s1, bool attack) int berserk; int strongmen; int speeded = 0, speed = 1; - bool pr_aid = false; int rest; const group *g = NULL; const attrib *a = a_find(u->attribs, &at_otherfaction); @@ -3348,14 +3347,6 @@ fighter *make_fighter(battle * b, unit * u, side * s1, bool attack) if (i < berserk) { fig->person[i].attack++; } - /* Leute mit einem Aid-Prayer bekommen +1 auf fast alles. */ - if (pr_aid) { - fig->person[i].attack++; - fig->person[i].defence++; - fig->person[i].damage++; - fig->person[i].damage_rear++; - fig->person[i].flags |= FL_COURAGE; - } /* Leute mit Kraftzauber machen +2 Schaden im Nahkampf. */ if (i < strongmen) { fig->person[i].damage += 2; From c090f8e1f4609baed38149220685d08383eb9cea Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 12:03:26 +0100 Subject: [PATCH 08/15] CID 22441 Unchecked return value from library --- src/battle.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/battle.c b/src/battle.c index 2e99c60b1..97afb3de4 100644 --- a/src/battle.c +++ b/src/battle.c @@ -3622,18 +3622,22 @@ battle *make_battle(region * r) char zText[MAX_PATH]; char zFilename[MAX_PATH]; sprintf(zText, "%s/battles", basepath()); - _mkdir(zText); - sprintf(zFilename, "%s/battle-%d-%s.log", zText, obs_count, simplename(r)); - bdebug = fopen(zFilename, "w"); - if (!bdebug) - log_error("battles cannot be debugged\n"); - else { - const unsigned char utf8_bom[4] = { 0xef, 0xbb, 0xbf, 0 }; - fwrite(utf8_bom, 1, 3, bdebug); - fprintf(bdebug, "In %s findet ein Kampf statt:\n", rname(r, - default_locale)); + if (_mkdir(zText) != 0) { + log_error("could not create subdirectory for battle logs: %s", zText); + battledebug = false; + } + else { + sprintf(zFilename, "%s/battle-%d-%s.log", zText, obs_count++, simplename(r)); + bdebug = fopen(zFilename, "w"); + if (!bdebug) + log_error("battles cannot be debugged"); + else { + const unsigned char utf8_bom[4] = { 0xef, 0xbb, 0xbf, 0 }; + fwrite(utf8_bom, 1, 3, bdebug); + fprintf(bdebug, "In %s findet ein Kampf statt:\n", rname(r, + default_locale)); + } } - obs_count++; } b->region = r; From 9d4489972baec72fe75431c4d447e7950ca64628 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 12:09:45 +0100 Subject: [PATCH 09/15] CID 22447 Calling risky function github issue #336 --- src/laws.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/laws.c b/src/laws.c index 3b4cc6fca..3aa9f8130 100755 --- a/src/laws.c +++ b/src/laws.c @@ -4525,7 +4525,7 @@ void update_subscriptions(void) int subscription, fno; faction *f; - if (fscanf(F, "%d %s", &subscription, zFaction) <= 0) + if (fscanf(F, "%d %4s", &subscription, zFaction) <= 0) break; fno = atoi36(zFaction); f = findfaction(fno); From 657f5044e59ca982c9311cd7a5bb26c73a70c6ea Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 12:11:26 +0100 Subject: [PATCH 10/15] CID 22495 Identical code for different branches --- src/report.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/report.c b/src/report.c index a8a347689..9f6e28fd7 100644 --- a/src/report.c +++ b/src/report.c @@ -2361,11 +2361,6 @@ const char *charset) newline(out); write_travelthru(out, r, f); } - else if (sr->mode == see_lighthouse) { - describe(out, sr, f); - newline(out); - write_travelthru(out, r, f); - } else { describe(out, sr, f); newline(out); From 1a715d6736bf1e5429356eb53aeba7c03a180d6a Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 12:14:12 +0100 Subject: [PATCH 11/15] CID 22493 Dereference after null check --- src/report.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/report.c b/src/report.c index 9f6e28fd7..50a17dffb 100644 --- a/src/report.c +++ b/src/report.c @@ -1848,18 +1848,16 @@ const faction * f) { int i, bytes; const char *name, *bname, *billusion = NULL; - const struct locale *lang = NULL; + const struct locale *lang; char buffer[8192], *bufp = buffer; message *msg; size_t size = sizeof(buffer) - 1; + assert(f); + lang = f->locale; newline(out); - - if (f) - lang = f->locale; - bytes = - _snprintf(bufp, size, "%s, %s %d, ", buildingname(b), LOC(f->locale, + _snprintf(bufp, size, "%s, %s %d, ", buildingname(b), LOC(lang, "nr_size"), b->size); if (wrptr(&bufp, &size, bytes) != 0) WARN_STATIC_BUFFER(); @@ -1881,7 +1879,7 @@ const faction * f) } if (b->size < b->type->maxsize) { - bytes = (int)strlcpy(bufp, LOC(f->locale, "nr_building_inprogress"), size); + bytes = (int)strlcpy(bufp, LOC(lang, "nr_building_inprogress"), size); if (wrptr(&bufp, &size, bytes) != 0) WARN_STATIC_BUFFER(); } @@ -1889,7 +1887,7 @@ const faction * f) if (b->besieged > 0 && sr->mode >= see_lighthouse) { msg = msg_message("nr_building_besieged", "soldiers diff", b->besieged, b->besieged - b->size * SIEGEFACTOR); - bytes = (int)nr_render(msg, f->locale, bufp, size, f); + bytes = (int)nr_render(msg, lang, bufp, size, f); if (wrptr(&bufp, &size, bytes) != 0) WARN_STATIC_BUFFER(); msg_release(msg); @@ -1926,6 +1924,7 @@ static void nr_paragraph(stream *out, message * m, faction * f) char buf[4096], *bufp = buf; size_t size = sizeof(buf) - 1; + assert(f); bytes = (int)nr_render(m, f->locale, bufp, size, f); if (wrptr(&bufp, &size, bytes) != 0) WARN_STATIC_BUFFER(); From b7b1ae43c268203b131797bb92c9a8b32f96d8fd Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 12:15:25 +0100 Subject: [PATCH 12/15] CID 22493 Dereference after null check CID 22458 Logically dead code github issue #361 --- src/report.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/report.c b/src/report.c index 50a17dffb..f157a75f4 100644 --- a/src/report.c +++ b/src/report.c @@ -2330,7 +2330,7 @@ const char *charset) message *m = 0; if (herb && lux) { m = msg_message("nr_market_info_p", "p1 p2", - lux ? lux->rtype : 0, herb ? herb->rtype : 0); + lux->rtype, herb->rtype); } else if (lux || herb) { m = msg_message("nr_market_info_s", "p1", From 5887e8e48feef84f85490fc71728c7ff3f7462ff Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 12:19:16 +0100 Subject: [PATCH 13/15] CID 22434 Ignoring number of bytes read --- src/kernel/jsonconf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/kernel/jsonconf.c b/src/kernel/jsonconf.c index 9e23c3360..4602008f5 100644 --- a/src/kernel/jsonconf.c +++ b/src/kernel/jsonconf.c @@ -839,8 +839,9 @@ static void json_include(cJSON *json) { fseek(F, 0, SEEK_END); sz = ftell(F); rewind(F); - data = malloc(sz); - fread(data, 1, sz, F); + data = malloc(sz+1); + sz = fread(data, 1, sz, F); + data[sz] = 0; fclose(F); config = cJSON_Parse(data); free(data); From 8bc44d82edd64ba7db4984b81d9cbc3751f5435f Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 12:29:51 +0100 Subject: [PATCH 14/15] CID 22500 Argument cannot be negative --- src/kernel/jsonconf.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/kernel/jsonconf.c b/src/kernel/jsonconf.c index 4602008f5..5fc3eea34 100644 --- a/src/kernel/jsonconf.c +++ b/src/kernel/jsonconf.c @@ -833,25 +833,29 @@ static void json_include(cJSON *json) { F = fopen(child->valuestring, "rt"); } if (F) { - cJSON *config; - char *data; - size_t sz; + long pos; fseek(F, 0, SEEK_END); - sz = ftell(F); + pos = ftell(F); rewind(F); - data = malloc(sz+1); - sz = fread(data, 1, sz, F); - data[sz] = 0; + if (pos > 0) { + cJSON *config; + char *data; + size_t sz; + + data = malloc(pos + 1); + sz = fread(data, 1, (size_t)pos, F); + data[sz] = 0; + config = cJSON_Parse(data); + free(data); + if (config) { + json_config(config); + cJSON_Delete(config); + } + else { + log_error("invalid JSON, could not parse %s", child->valuestring); + } + } fclose(F); - config = cJSON_Parse(data); - free(data); - if (config) { - json_config(config); - cJSON_Delete(config); - } - else { - log_error("invalid JSON, could not parse %s", child->valuestring); - } } } } From 45b6de9c0d1fab8a272bb7fecffe0d3fc1033a2d Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 4 Nov 2015 12:30:46 +0100 Subject: [PATCH 15/15] CID 22468 Division or modulo by zero --- src/names.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/names.c b/src/names.c index e32d553d7..de0a81263 100644 --- a/src/names.c +++ b/src/names.c @@ -91,7 +91,7 @@ static const char *make_names(const char *monster, int *num_postfix, uu = rng_int() % *num_name; /* nur 50% aller Namen haben "Nach-Teil", wenn kein Vor-Teil */ - if (uv >= *num_prefix) { + if (*num_prefix > 0 && uv >= *num_prefix) { un = rng_int() % *num_postfix; } else {