Req->Bug #67343 [Opn->Nab]: date/lib/parse_date.c does not check *allocs

From: Date: Mon, 16 Jul 2018 13:32:01 +0000
Subject: Req->Bug #67343 [Opn->Nab]: date/lib/parse_date.c does not check *allocs
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-216351@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=67343&edit=1 ID: 67343 Updated by: cmb@php.net Reported by: al-phpbug at none dot at Summary: date/lib/parse_date.c does not check *allocs -Status: Open +Status: Not a bug -Type: Feature/Change Request +Type: Bug Package: Date/time related Operating System: all PHP Version: 5.5.12 -Assigned To: +Assigned To: cmb Block user comment: N Private report: N New Comment: Generally, not checking the return value of malloc() and friends has to be regarded as bug. However, as of commit c68f45b[1] timelib introduced timelib_malloc and friends, and these are defined as emalloc etc. for PHP[2]. There are still some occurrences of malloc() in timelib, but this is an upstream issue[3]. [1] <https://github.com/derickr/timelib/commit/c68f45bb6860e671f2d310b44e54656df0679c05> [2] <https://github.com/php/php-src/blob/php-7.2.7/ext/date/config0.m4#L31-L35> <https://github.com/php/php-src/blob/php-7.2.7/ext/date/config.w32#L16-L20> [3] <https://github.com/derickr/timelib/issues/41> Previous Comments: ------------------------------------------------------------------------ [2014-05-26 12:52:41] al-phpbug at none dot at Description: ------------ According to point 9 of php-5.5.12/CODING_STANDARDS the parse_date does not use the emalloc(), efree(), estrdup(), etc. and it does not check if the *alloc was successfully egrep -A3 alloc php-5.5.12/ext/date/lib/parse_date.c ####### uchar *buf = (uchar*) malloc(((s->lim - s->bot) + BSIZE)*sizeof(uchar)); memcpy(buf, s->tok, s->lim - s->tok); s->tok = buf; s->ptr = &buf[s->ptr - s->bot]; -- s->errors->warning_messages = realloc(s->errors->warning_messages, s->errors->warning_count * sizeof(timelib_error_message)); s->errors->warning_messages[s->errors->warning_count - 1].position = s->tok ? s->tok - s->str : 0; s->errors->warning_messages[s->errors->warning_count - 1].character = s->tok ? *s->tok : 0; s->errors->warning_messages[s->errors->warning_count - 1].message = strdup(error); -- s->errors->error_messages = realloc(s->errors->error_messages, s->errors->error_count * sizeof(timelib_error_message)); s->errors->error_messages[s->errors->error_count - 1].position = s->tok ? s->tok - s->str : 0; s->errors->error_messages[s->errors->error_count - 1].character = s->tok ? *s->tok : 0; s->errors->error_messages[s->errors->error_count - 1].message = strdup(error); -- s->errors->warning_messages = realloc(s->errors->warning_messages, s->errors->warning_count * sizeof(timelib_error_message)); s->errors->warning_messages[s->errors->warning_count - 1].position = cptr - sptr; s->errors->warning_messages[s->errors->warning_count - 1].character = *cptr; s->errors->warning_messages[s->errors->warning_count - 1].message = strdup(error); -- s->errors->error_messages = realloc(s->errors->error_messages, s->errors->error_count * sizeof(timelib_error_message)); s->errors->error_messages[s->errors->error_count - 1].position = cptr - sptr; s->errors->error_messages[s->errors->error_count - 1].character = *cptr; s->errors->error_messages[s->errors->error_count - 1].message = strdup(error); -- char *tmp = calloc(1, s->cur - s->tok + 1); memcpy(tmp, s->tok, s->cur - s->tok); return tmp; -- str = calloc(1, end - begin + 1); memcpy(str, begin, end - begin); tmp_nr = strtoll(str, NULL, 10); free(str); -- str = calloc(1, end - begin + 1); memcpy(str, begin, end - begin); if (str[0] == ':') { str[0] = '.'; -- word = calloc(1, end - begin + 1); memcpy(word, begin, end - begin); for (tp = timelib_reltext_lookup; tp->name; tp++) { -- word = calloc(1, end - begin + 1); memcpy(word, begin, end - begin); for (tp = timelib_month_lookup; tp->name; tp++) { -- word = calloc(1, end - begin + 1); memcpy(word, begin, end - begin); for (tp = timelib_relunit_lookup; tp->name; tp++) { -- word = calloc(1, end - begin + 1); memcpy(word, begin, end - begin); if ((tp = zone_search(word, -1, 0))) { -- in.errors = malloc(sizeof(struct timelib_error_container)); in.errors->warning_count = 0; in.errors->warning_messages = NULL; in.errors->error_count = 0; -- in.str = malloc((e - s) + YYMAXFILL); memset(in.str, 0, (e - s) + YYMAXFILL); memcpy(in.str, s, (e - s)); in.lim = in.str + (e - s) + YYMAXFILL; -- in.errors = malloc(sizeof(struct timelib_error_container)); in.errors->warning_count = 0; in.errors->warning_messages = NULL; in.errors->error_count = 0; ####### Maybe it is unimportant but it would be nice if anyone take a look there and say it's ok or not. I think the whole date directory should be reviewed. egrep -r -A3 alloc php-5.5.12/ext/date/* ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=67343&edit=1

« previous php.bugs (#216351) next »