From 4122b14b3a0d51dbfc89274346ff12bd96f34acb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 8 Apr 2019 00:18:54 +0900 Subject: [PATCH 1/4] calendarspec: use structured initializers --- src/shared/calendarspec.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c index 601bde6d4d..f6d13a53ff 100644 --- a/src/shared/calendarspec.c +++ b/src/shared/calendarspec.c @@ -551,14 +551,16 @@ static int const_chain(int value, CalendarComponent **c) { assert(c); - cc = new0(CalendarComponent, 1); + cc = new(CalendarComponent, 1); if (!cc) return -ENOMEM; - cc->start = value; - cc->stop = -1; - cc->repeat = 0; - cc->next = *c; + *cc = (CalendarComponent) { + .start = value, + .stop = -1, + .repeat = 0, + .next = *c, + }; *c = cc; @@ -644,14 +646,16 @@ static int prepend_component(const char **p, bool usec, unsigned nesting, Calend if (!IN_SET(*e, 0, ' ', ',', '-', '~', ':')) return -EINVAL; - cc = new0(CalendarComponent, 1); + cc = new(CalendarComponent, 1); if (!cc) return -ENOMEM; - cc->start = start; - cc->stop = stop; - cc->repeat = repeat; - cc->next = *c; + *cc = (CalendarComponent) { + .start = start, + .stop = stop, + .repeat = repeat, + .next = *c, + }; *p = e; *c = cc; @@ -888,11 +892,14 @@ int calendar_spec_from_string(const char *p, CalendarSpec **spec) { assert(p); assert(spec); - c = new0(CalendarSpec, 1); + c = new(CalendarSpec, 1); if (!c) return -ENOMEM; - c->dst = -1; - c->timezone = NULL; + + *c = (CalendarSpec) { + .dst = -1, + .timezone = NULL, + }; utc = endswith_no_case(p, " UTC"); if (utc) { From 9eef82e5a8b51befa05bef07425b4d5ba49b3610 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 8 Apr 2019 00:21:37 +0900 Subject: [PATCH 2/4] calendarspec: rename free_chain() to chain_free() --- src/shared/calendarspec.c | 48 +++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c index f6d13a53ff..00eb1cf31a 100644 --- a/src/shared/calendarspec.c +++ b/src/shared/calendarspec.c @@ -33,7 +33,7 @@ * linked compenents anyway. */ #define CALENDARSPEC_COMPONENTS_MAX 240 -static void free_chain(CalendarComponent *c) { +static void chain_free(CalendarComponent *c) { CalendarComponent *n; while (c) { @@ -48,12 +48,12 @@ CalendarSpec* calendar_spec_free(CalendarSpec *c) { if (!c) return NULL; - free_chain(c->year); - free_chain(c->month); - free_chain(c->day); - free_chain(c->hour); - free_chain(c->minute); - free_chain(c->microsecond); + chain_free(c->year); + chain_free(c->month); + chain_free(c->day); + chain_free(c->hour); + chain_free(c->minute); + chain_free(c->microsecond); free(c->timezone); return mfree(c); @@ -568,8 +568,8 @@ static int const_chain(int value, CalendarComponent **c) { } static int calendarspec_from_time_t(CalendarSpec *c, time_t time) { + _cleanup_CalendarComponent *year = NULL, *month = NULL, *day = NULL, *hour = NULL, *minute = NULL, *us = NULL; struct tm tm; - CalendarComponent *year = NULL, *month = NULL, *day = NULL, *hour = NULL, *minute = NULL, *us = NULL; int r; if (!gmtime_r(&time, &tm)) @@ -693,7 +693,7 @@ static int parse_chain(const char **p, bool usec, CalendarComponent **c) { r = prepend_component(&t, usec, 0, &cc); if (r < 0) { - free_chain(cc); + chain_free(cc); return r; } @@ -743,21 +743,21 @@ static int parse_date(const char **p, CalendarSpec *c) { /* Already the end? A ':' as separator? In that case this was a time, not a date */ if (IN_SET(*t, 0, ':')) { - free_chain(first); + chain_free(first); return 0; } if (*t == '~') c->end_of_month = true; else if (*t != '-') { - free_chain(first); + chain_free(first); return -EINVAL; } t++; r = parse_chain(&t, false, &second); if (r < 0) { - free_chain(first); + chain_free(first); return r; } @@ -768,24 +768,24 @@ static int parse_date(const char **p, CalendarSpec *c) { c->day = second; return 0; } else if (c->end_of_month) { - free_chain(first); - free_chain(second); + chain_free(first); + chain_free(second); return -EINVAL; } if (*t == '~') c->end_of_month = true; else if (*t != '-') { - free_chain(first); - free_chain(second); + chain_free(first); + chain_free(second); return -EINVAL; } t++; r = parse_chain(&t, false, &third); if (r < 0) { - free_chain(first); - free_chain(second); + chain_free(first); + chain_free(second); return r; } @@ -798,9 +798,9 @@ static int parse_date(const char **p, CalendarSpec *c) { return 0; } - free_chain(first); - free_chain(second); - free_chain(third); + chain_free(first); + chain_free(second); + chain_free(third); return -EINVAL; } @@ -877,9 +877,9 @@ finish: return 0; fail: - free_chain(h); - free_chain(m); - free_chain(s); + chain_free(h); + chain_free(m); + chain_free(s); return r; } From fb3ba5ec119ba067e01dde2672d276ed48d2c366 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 8 Apr 2019 00:32:14 +0900 Subject: [PATCH 3/4] calendarspec: use _cleanup_ attributes for CalendarComponent --- src/shared/calendarspec.c | 120 +++++++++++++++----------------------- 1 file changed, 46 insertions(+), 74 deletions(-) diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c index 00eb1cf31a..7caf7c64c3 100644 --- a/src/shared/calendarspec.c +++ b/src/shared/calendarspec.c @@ -43,6 +43,8 @@ static void chain_free(CalendarComponent *c) { } } +DEFINE_TRIVIAL_CLEANUP_FUNC(CalendarComponent*, chain_free); + CalendarSpec* calendar_spec_free(CalendarSpec *c) { if (!c) @@ -568,7 +570,9 @@ static int const_chain(int value, CalendarComponent **c) { } static int calendarspec_from_time_t(CalendarSpec *c, time_t time) { - _cleanup_CalendarComponent *year = NULL, *month = NULL, *day = NULL, *hour = NULL, *minute = NULL, *us = NULL; + _cleanup_(chain_freep) CalendarComponent + *year = NULL, *month = NULL, *day = NULL, + *hour = NULL, *minute = NULL, *us = NULL; struct tm tm; int r; @@ -600,12 +604,12 @@ static int calendarspec_from_time_t(CalendarSpec *c, time_t time) { return r; c->utc = true; - c->year = year; - c->month = month; - c->day = day; - c->hour = hour; - c->minute = minute; - c->microsecond = us; + c->year = TAKE_PTR(year); + c->month = TAKE_PTR(month); + c->day = TAKE_PTR(day); + c->hour = TAKE_PTR(hour); + c->minute = TAKE_PTR(minute); + c->microsecond = TAKE_PTR(us); return 0; } @@ -669,8 +673,8 @@ static int prepend_component(const char **p, bool usec, unsigned nesting, Calend } static int parse_chain(const char **p, bool usec, CalendarComponent **c) { + _cleanup_(chain_freep) CalendarComponent *cc = NULL; const char *t; - CalendarComponent *cc = NULL; int r; assert(p); @@ -692,20 +696,18 @@ static int parse_chain(const char **p, bool usec, CalendarComponent **c) { } r = prepend_component(&t, usec, 0, &cc); - if (r < 0) { - chain_free(cc); + if (r < 0) return r; - } *p = t; - *c = cc; + *c = TAKE_PTR(cc); return 0; } static int parse_date(const char **p, CalendarSpec *c) { + _cleanup_(chain_freep) CalendarComponent *first = NULL, *second = NULL, *third = NULL; const char *t; int r; - CalendarComponent *first, *second, *third; assert(p); assert(*p); @@ -742,70 +744,51 @@ static int parse_date(const char **p, CalendarSpec *c) { return r; /* Already the end? A ':' as separator? In that case this was a time, not a date */ - if (IN_SET(*t, 0, ':')) { - chain_free(first); + if (IN_SET(*t, 0, ':')) return 0; - } if (*t == '~') c->end_of_month = true; - else if (*t != '-') { - chain_free(first); + else if (*t != '-') return -EINVAL; - } t++; r = parse_chain(&t, false, &second); - if (r < 0) { - chain_free(first); + if (r < 0) return r; - } /* Got two parts, hence it's month and day */ if (IN_SET(*t, 0, ' ')) { *p = t + strspn(t, " "); - c->month = first; - c->day = second; + c->month = TAKE_PTR(first); + c->day = TAKE_PTR(second); return 0; - } else if (c->end_of_month) { - chain_free(first); - chain_free(second); + } else if (c->end_of_month) return -EINVAL; - } if (*t == '~') c->end_of_month = true; - else if (*t != '-') { - chain_free(first); - chain_free(second); + else if (*t != '-') return -EINVAL; - } t++; r = parse_chain(&t, false, &third); - if (r < 0) { - chain_free(first); - chain_free(second); + if (r < 0) return r; - } + + if (!IN_SET(*t, 0, ' ')) + return -EINVAL; /* Got three parts, hence it is year, month and day */ - if (IN_SET(*t, 0, ' ')) { - *p = t + strspn(t, " "); - c->year = first; - c->month = second; - c->day = third; - return 0; - } - - chain_free(first); - chain_free(second); - chain_free(third); - return -EINVAL; + *p = t + strspn(t, " "); + c->year = TAKE_PTR(first); + c->month = TAKE_PTR(second); + c->day = TAKE_PTR(third); + return 0; } static int parse_calendar_time(const char **p, CalendarSpec *c) { - CalendarComponent *h = NULL, *m = NULL, *s = NULL; + _cleanup_(chain_freep) CalendarComponent *h = NULL, *m = NULL, *s = NULL; const char *t; int r; @@ -821,66 +804,55 @@ static int parse_calendar_time(const char **p, CalendarSpec *c) { r = parse_chain(&t, false, &h); if (r < 0) - goto fail; + return r; - if (*t != ':') { - r = -EINVAL; - goto fail; - } + if (*t != ':') + return -EINVAL; t++; r = parse_chain(&t, false, &m); if (r < 0) - goto fail; + return r; /* Already at the end? Then it's hours and minutes, and seconds are 0 */ if (*t == 0) goto null_second; - if (*t != ':') { - r = -EINVAL; - goto fail; - } + if (*t != ':') + return -EINVAL; t++; r = parse_chain(&t, true, &s); if (r < 0) - goto fail; + return r; /* At the end? Then it's hours, minutes and seconds */ if (*t == 0) goto finish; - r = -EINVAL; - goto fail; + return -EINVAL; null_hour: r = const_chain(0, &h); if (r < 0) - goto fail; + return r; r = const_chain(0, &m); if (r < 0) - goto fail; + return r; null_second: r = const_chain(0, &s); if (r < 0) - goto fail; + return r; finish: *p = t; - c->hour = h; - c->minute = m; - c->microsecond = s; + c->hour = TAKE_PTR(h); + c->minute = TAKE_PTR(m); + c->microsecond = TAKE_PTR(s); return 0; - -fail: - chain_free(h); - chain_free(m); - chain_free(s); - return r; } int calendar_spec_from_string(const char *p, CalendarSpec **spec) { From daa4aca1cb45445fb19b3711538f0576a2b1c346 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 8 Apr 2019 00:37:31 +0900 Subject: [PATCH 4/4] calendarspec: fix possible integer overflow Fixes oss-fuzz#14108. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=14108 --- src/shared/calendarspec.c | 3 +++ test/fuzz/fuzz-calendarspec/oss-fuzz-14108 | 1 + 2 files changed, 4 insertions(+) create mode 100644 test/fuzz/fuzz-calendarspec/oss-fuzz-14108 diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c index 7caf7c64c3..d83e7962a6 100644 --- a/src/shared/calendarspec.c +++ b/src/shared/calendarspec.c @@ -579,6 +579,9 @@ static int calendarspec_from_time_t(CalendarSpec *c, time_t time) { if (!gmtime_r(&time, &tm)) return -ERANGE; + if (tm.tm_year > INT_MAX - 1900) + return -ERANGE; + r = const_chain(tm.tm_year + 1900, &year); if (r < 0) return r; diff --git a/test/fuzz/fuzz-calendarspec/oss-fuzz-14108 b/test/fuzz/fuzz-calendarspec/oss-fuzz-14108 new file mode 100644 index 0000000000..6899c23a7e --- /dev/null +++ b/test/fuzz/fuzz-calendarspec/oss-fuzz-14108 @@ -0,0 +1 @@ +@67767992554749550 \ No newline at end of file