From: Andre Noll Date: Mon, 29 Jan 2018 22:21:11 +0000 (+0100) Subject: server: Avoid use of uninitialized memory. X-Git-Tag: v0.6.2~34 X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=4c117836f38d3cc661f6271760ee5cab78ada8cf;hp=bde44cf34bd9b334892af6ef4731ecb74b1ab544 server: Avoid use of uninitialized memory. change_current_mood() receives an errmsg pointer which the callers expect to be initialized with an error string if (and only if) the function returns negative. However, most error paths miss to initialize the pointer which results in undefined behaviour in the caller which attempts to free(3) uninitialized memory. The gcc AddressSanitizer and valgrind both catch this: gcc: ==14788==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x081af250 in thread T0 valgrind: ==4410== Invalid free() / delete / delete[] / realloc() The bug was introduced half a year ago when version 2 moods were introduced in commit 3d3a2f50. --- diff --git a/mood.c b/mood.c index ae8364a2..a63d4d2a 100644 --- a/mood.c +++ b/mood.c @@ -418,10 +418,13 @@ static int load_mood(const struct osl_row *mood_row, struct mood **m, *m = NULL; ret = mood_get_name_and_def_by_row(mood_row, &mood_name, &mood_def); - if (ret < 0) + if (ret < 0) { + if (errmsg) + *errmsg = make_message( + "could not read mood definition"); return ret; - if (!*mood_name) - return -E_DUMMY_ROW; + } + assert(*mood_name); mlpd.m = alloc_new_mood(mood_name); ret = for_each_line(FELF_READ_ONLY, mood_def.data, mood_def.size, parse_mood_line, &mlpd); @@ -876,7 +879,9 @@ int change_current_mood(const char *mood_name, char **errmsg) }; ret = osl(osl_get_row(moods_table, BLOBCOL_NAME, &obj, &row)); if (ret < 0) { - PARA_NOTICE_LOG("no such mood: %s\n", mood_name); + if (errmsg) + *errmsg = make_message("no such mood: %s", + mood_name); return ret; } ret = load_mood(row, &m, errmsg); @@ -891,13 +896,20 @@ int change_current_mood(const char *mood_name, char **errmsg) aa.m = current_mood; PARA_NOTICE_LOG("computing statistics of admissible files\n"); ret = audio_file_loop(&aa, add_if_admissible); - if (ret < 0) + if (ret < 0) { + if (errmsg) + *errmsg = make_message("audio file loop failed"); return ret; + } for (i = 0; i < statistics.num; i++) { struct admissible_file_info *a = aa.array + i; ret = add_to_score_table(a->aft_row, a->score); - if (ret < 0) + if (ret < 0) { + if (errmsg) + *errmsg = make_message( + "could not add row to score table"); goto out; + } } log_statistics(); ret = statistics.num;