Merge branch 'asctime-ub-asserts' into 'master'

Catch UB in asctime_r()

See merge request redox-os/relibc!312

src/header/time/mod.rs

@@ -79,24 +79,75 @@ pub unsafe extern "C" fn asctime(timeptr: *const tm) -> *mut c_char {
 
 #[no_mangle]
 pub unsafe extern "C" fn asctime_r(tm: *const tm, buf: *mut c_char) -> *mut c_char {
-    let tm = &*tm;
-    let result = core::fmt::write(
-        &mut platform::UnsafeStringWriter(buf as *mut u8),
+    let tm_sec = (*tm).tm_sec;
+    let tm_min = (*tm).tm_min;
+    let tm_hour = (*tm).tm_hour;
+    let tm_mday = (*tm).tm_mday;
+    let tm_mon = (*tm).tm_mon;
+    let tm_year = (*tm).tm_year;
+    let tm_wday = (*tm).tm_wday;
+
+    /* Panic when we run into undefined behavior.
+     *
+     * POSIX says (since issue 7) that asctime()/asctime_r() cause UB
+     * when the tm member values would cause out-of-bounds array access
+     * or overflow the output buffer. This contrasts with ISO C11+,
+     * which specifies UB for any tm members being outside their normal
+     * ranges. While POSIX explicitly defers to the C standard in case
+     * of contradictions, the assertions below follow the interpretation
+     * that POSIX simply defines some of C's undefined behavior, rather
+     * than conflict with the ISO standard.
+     *
+     * Note that C's "%.2d" formatting, unlike Rust's "{:02}"
+     * formatting, does not count a minus sign against the two digits to
+     * print, meaning that we must reject all negative values for
+     * seconds, minutes and hours. However, C's "%3d" (for day-of-month)
+     * is similar to Rust's "{:3}".
+     *
+     * To avoid year overflow problems (in Rust, where numeric overflow
+     * is considered an error), we subtract 1900 from the endpoints,
+     * rather than adding to the tm_year value. POSIX' requirement that
+     * tm_year be at most {INT_MAX}-1990 is satisfied for all legal
+     * values of {INT_MAX} through the max-4-digit requirement on the
+     * year.
+     *
+     * The tm_mon and tm_wday fields are used for array access and thus
+     * will already cause a panic in Rust code when out of range.
+     * However, using the assertions below allows a consistent error
+     * message for all fields. */
+    const OUT_OF_RANGE_MESSAGE: &str = "tm member out of range";
+
+    assert!(0 <= tm_sec && tm_sec <= 99, OUT_OF_RANGE_MESSAGE);
+    assert!(0 <= tm_min && tm_min <= 99, OUT_OF_RANGE_MESSAGE);
+    assert!(0 <= tm_hour && tm_hour <= 99, OUT_OF_RANGE_MESSAGE);
+    assert!(-99 <= tm_mday && tm_mday <= 999, OUT_OF_RANGE_MESSAGE);
+    assert!(0 <= tm_mon && tm_mon <= 11, OUT_OF_RANGE_MESSAGE);
+    assert!(
+        -999 - 1900 <= tm_year && tm_year <= 9999 - 1900,
+        OUT_OF_RANGE_MESSAGE
+    );
+    assert!(0 <= tm_wday && tm_wday <= 6, OUT_OF_RANGE_MESSAGE);
+
+    // At this point, we can safely use the values as given.
+    let write_result = core::fmt::write(
+        // buf may be either `*mut u8` or `*mut i8`
+        &mut platform::UnsafeStringWriter(buf.cast()),
         format_args!(
             "{:.3} {:.3}{:3} {:02}:{:02}:{:02} {}\n",
-            DAY_NAMES[tm.tm_wday as usize],
-            MON_NAMES[tm.tm_mon as usize],
-            tm.tm_mday as usize,
-            tm.tm_hour as usize,
-            tm.tm_min as usize,
-            tm.tm_sec as usize,
-            (1900 + tm.tm_year)
+            DAY_NAMES[usize::try_from(tm_wday).unwrap()],
+            MON_NAMES[usize::try_from(tm_mon).unwrap()],
+            tm_mday,
+            tm_hour,
+            tm_min,
+            tm_sec,
+            1900 + tm_year
         ),
     );
-    match result {
+    match write_result {
         Ok(_) => buf,
         Err(_) => {
-            platform::errno = EIO;
+            /* asctime()/asctime_r() or the equivalent sprintf() call
+             * have no defined errno setting */
             core::ptr::null_mut()
         }
     }

tests/expected/time/asctime.stdout

@@ -0,0 +1,5 @@
+Thu Jan  1 00:00:00 1970
+Sun Jan  1 00:00:00 1000
+Sat Dec 31 23:59:60 9999
+Sun Jan-99 00:00:00 -999
+Sat Dec999 99:99:99 9999

tests/time/asctime.c

@@ -6,12 +6,34 @@
 #include "test_helpers.h"
 
 int main(void) {
-    time_t a = 0;
-    struct tm *time_info = gmtime(&a);
+    time_t unix_epoch = 0;
+    struct tm *unix_epoch_tm_ptr = gmtime(&unix_epoch);
 
-    char *time_string = asctime(time_info);
+    char *time_string = NULL;
 
-    if (time_string == NULL || strcmp(time_string, "Thu Jan  1 00:00:00 1970\n") != 0) {
-        exit(EXIT_FAILURE);
-    }
+    /* Min/max non-UB-causing values according to ISO C11 and newer. */
+    struct tm iso_c_min_tm = {.tm_sec = 0, .tm_min = 0, .tm_hour = 0, .tm_mday = 1, .tm_mon = 0, .tm_year = 1000-1900, .tm_wday = 0, .tm_yday = 0, .tm_isdst = 0, .tm_gmtoff = 0, .tm_zone = NULL};
+    struct tm iso_c_max_tm = {.tm_sec = 60, .tm_min = 59, .tm_hour = 23, .tm_mday = 31, .tm_mon = 11, .tm_year = 9999-1900, .tm_wday = 6, .tm_yday = 0, .tm_isdst = 0, .tm_gmtoff = 0, .tm_zone = NULL};
+
+    /* Min/max non-UB-causing values according to POSIX (issue 7). These
+     * will cause UB according to the ISO standard! */
+    struct tm posix_min_tm = {.tm_sec = 0, .tm_min = 0, .tm_hour = 0, .tm_mday = -99, .tm_mon = 0, .tm_year = -999-1900, .tm_wday = 0, .tm_yday = 0, .tm_isdst = 0, .tm_gmtoff = 0, .tm_zone = NULL};
+    struct tm posix_max_tm = {.tm_sec = 99, .tm_min = 99, .tm_hour = 99, .tm_mday = 999, .tm_mon = 11, .tm_year = 9999-1900, .tm_wday = 6, .tm_yday = 0, .tm_isdst = 0, .tm_gmtoff = 0, .tm_zone = NULL};
+
+    time_string = asctime(unix_epoch_tm_ptr);
+    printf("%s", time_string);
+
+    time_string = asctime(&iso_c_min_tm);
+    printf("%s", time_string);
+
+    time_string = asctime(&iso_c_max_tm);
+    printf("%s", time_string);
+
+    time_string = asctime(&posix_min_tm);
+    printf("%s", time_string);
+
+    time_string = asctime(&posix_max_tm);
+    printf("%s", time_string);
+
+    return 0;
 }