From 9c0565b2c3dec698a526cade57b331004699e094 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 19 May 2017 14:49:06 -0400
Subject: [PATCH] basic/time-util: make parsing of dual_timestamp more strict

*scanf functions set errno on i/o error. For sscanf, this doesn't really apply,
so (based on the man page), it seems that errno is unlikely to be ever set to a
useful value. So just ignore errno. The error message includes the string that
was parsed, so it should be always pretty clear why parsing failed.

On the other hand, detect trailing characters and minus prefix that weren't
converted properly. This matches what our safe_ato* functions do. Add tests to
elucidate various edge cases.
---
 src/basic/time-util.c | 18 ++++++++++++++++--
 src/test/test-time.c  | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/src/basic/time-util.c b/src/basic/time-util.c
index a0db97c41a7..8d55af84922 100644
--- a/src/basic/time-util.c
+++ b/src/basic/time-util.c
@@ -555,15 +555,29 @@ void dual_timestamp_serialize(FILE *f, const char *name, dual_timestamp *t) {
 
 int dual_timestamp_deserialize(const char *value, dual_timestamp *t) {
         uint64_t a, b;
+        int r, pos;
 
         assert(value);
         assert(t);
 
-        if (sscanf(value, "%" PRIu64 "%" PRIu64, &a, &b) != 2) {
-                log_debug("Failed to parse dual timestamp value \"%s\": %m", value);
+        pos = strspn(value, WHITESPACE);
+        if (value[pos] == '-')
+                return -EINVAL;
+        pos += strspn(value + pos, DIGITS);
+        pos += strspn(value + pos, WHITESPACE);
+        if (value[pos] == '-')
+                return -EINVAL;
+
+        r = sscanf(value, "%" PRIu64 "%" PRIu64 "%n", &a, &b, &pos);
+        if (r != 2) {
+                log_debug("Failed to parse dual timestamp value \"%s\".", value);
                 return -EINVAL;
         }
 
+        if (value[pos] != '\0')
+                /* trailing garbage */
+                return -EINVAL;
+
         t->realtime = a;
         t->monotonic = b;
 
diff --git a/src/test/test-time.c b/src/test/test-time.c
index 911282bf0c0..c9e31e90e12 100644
--- a/src/test/test-time.c
+++ b/src/test/test-time.c
@@ -273,6 +273,43 @@ static void test_format_timestamp_utc(void) {
         test_format_timestamp_utc_one(USEC_INFINITY, NULL);
 }
 
+static void test_dual_timestamp_deserialize(void) {
+        int r;
+        dual_timestamp t;
+
+        r = dual_timestamp_deserialize("1234 5678", &t);
+        assert_se(r == 0);
+        assert_se(t.realtime == 1234);
+        assert_se(t.monotonic == 5678);
+
+        r = dual_timestamp_deserialize("1234x 5678", &t);
+        assert_se(r == -EINVAL);
+
+        r = dual_timestamp_deserialize("1234 5678y", &t);
+        assert_se(r == -EINVAL);
+
+        r = dual_timestamp_deserialize("-1234 5678", &t);
+        assert_se(r == -EINVAL);
+
+        r = dual_timestamp_deserialize("1234 -5678", &t);
+        assert_se(r == -EINVAL);
+
+        /* Check that output wasn't modified. */
+        assert_se(t.realtime == 1234);
+        assert_se(t.monotonic == 5678);
+
+        r = dual_timestamp_deserialize("+123 567", &t);
+        assert_se(r == 0);
+        assert_se(t.realtime == 123);
+        assert_se(t.monotonic == 567);
+
+        /* Check that we get "infinity" on overflow. */
+        r = dual_timestamp_deserialize("18446744073709551617 0", &t);
+        assert_se(r == 0);
+        assert_se(t.realtime == USEC_INFINITY);
+        assert_se(t.monotonic == 0);
+}
+
 int main(int argc, char *argv[]) {
         uintmax_t x;
 
@@ -288,6 +325,7 @@ int main(int argc, char *argv[]) {
         test_usec_sub();
         test_format_timestamp();
         test_format_timestamp_utc();
+        test_dual_timestamp_deserialize();
 
         /* Ensure time_t is signed */
         assert_cc((time_t) -1 < (time_t) 1);