ITADN

TimeUtil::FromString accepts spec-violating Duration/Timestamp inputs (8 sub-bugs)

#27438Closedjortles 创建于 2026-05-18
J
jortlescommented
# TimeUtil::FromString accepts spec-violating Duration/Timestamp inputs; diverges from JSON parser ## Summary `TimeUtil::FromString` for both `Duration` and `Timestamp` accepts malformed inputs that produce protos failing `IsDurationValid`/`IsTimestampValid`. The companion JSON parser (`JsonStringToMessage`) correctly rejects all the same inputs, creating a cross-API divergence where two documented public APIs disagree on what constitutes a valid Duration/Timestamp string. 8 sub-bug classes identified (4 Duration, 4 Timestamp). ## Reproduction ```cpp #include <cstdio> #include <google/protobuf/duration.pb.h> #include <google/protobuf/util/time_util.h> namespace gpbutil = google::protobuf::util; int main() { google::protobuf::Duration d; // Class A: leading whitespace -- mismatched signs gpbutil::TimeUtil::FromString(" -3.54s", &d); printf("A: s=%lld n=%d\n", (long long)d.seconds(), d.nanos()); // Output: s=-3 n=540000000 (signs mismatch! nanos should be negative) // Class B: double minus gpbutil::TimeUtil::FromString("--3.54s", &d); printf("B: s=%lld n=%d\n", (long long)d.seconds(), d.nanos()); // Output: s=3 n=-540000000 (opposite sign mismatch) // Class C: >9 fractional digits gpbutil::TimeUtil::FromString("0.1234567890s", &d); printf("C: s=%lld n=%d\n", (long long)d.seconds(), d.nanos()); // Output: s=0 n=1234567890 (exceeds max 999999999) // Class D: out-of-range seconds gpbutil::TimeUtil::FromString("315576000001s", &d); printf("D: s=%lld n=%d\n", (long long)d.seconds(), d.nanos()); // Output: s=315576000001 (exceeds spec max 315576000000) // All return true (parse_ok) but produce spec-invalid Duration protos } ``` ### All 8 classes | Class | Type | Trigger | Effect | |-------|------|---------|--------| | A | Duration | Leading whitespace + negative (`" -3.54s"`) | Signs mismatch: seconds<0, nanos>0 | | B | Duration | Double minus (`"--3.54s"`) | Signs mismatch: seconds>0, nanos<0 | | C | Duration | >9 fractional digits (`"0.1234567890s"`) | nanos exceeds 999999999 | | D | Duration | seconds > 315576000000 (`"315576000001s"`) | Out-of-range seconds | | E | Timestamp | Year 0000 (`"0000-12-31T23:59:59Z"`) | seconds < kTimestampMinSeconds | | F | Timestamp | Year 10000 (`"10000-01-01T00:00:00Z"`) | seconds > kTimestampMaxSeconds | | G | Timestamp | 5-digit year (`"99999-01-01T00:00:00Z"`) | Wildly past spec max | | H | Timestamp | Negative year (`"-0001-01-01T00:00:00Z"`) | Not in RFC3339 spec | ### Cross-API divergence For every class A-H, `JsonStringToMessage` correctly rejects the input while `TimeUtil::FromString` accepts it. A system where one component uses the JSON path and another uses `TimeUtil::FromString` will disagree on validity of identical input strings. ## Impact - **Who**: Systems using Duration/Timestamp for security-relevant time bounds: gRPC deadlines, Envoy timeouts, retry intervals, IAM token TTLs, lease durations. - **Severity**: Low-Medium. No memory corruption. Invalid Duration values propagate through serialization silently. Class A is most dangerous: `seconds * 1e9 + nanos` gives `-3e9 + 540e6 = -2.46e9` instead of intended `-3.54e9` -- a 31% magnitude shift in time calculations. - **17 fuzzer crashes** collected (15 class A + 2 classes C/D from follow-up campaign). ## Suggested Fix Add a single post-parse validation check in each `TimeUtil::FromString` overload: ```cpp bool TimeUtil::FromString(absl::string_view value, Duration* d) { // ... existing parsing ... if (!IsDurationValid(*d)) { d->Clear(); return false; } return true; } bool TimeUtil::FromString(absl::string_view value, Timestamp* t) { // ... existing parsing ... if (!IsTimestampValid(*t)) { t->Clear(); return false; } return true; } ``` This single change fixes all 8 classes and closes the divergence with `JsonStringToMessage`. ## Environment - protobuf v36-dev, commit `86f66fe` - Compiler: clang 19.1.7, AFL++ + ASan - Harness: `pb_json_wkt_fuzzer` mode 1
关闭于 2026-05-18 1 条评论