Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 28, 2025

TimeBuf exposed a public Write implementation that allowed arbitrary bytes, while as_str() used unsafe UTF-8 conversion. This enabled creating invalid strings:

let mut buf = TimeBuf::default();
buf.write(&[0xff]).unwrap();  // non-UTF8 byte
println!("{}", buf.as_str());  // undefined behavior

Changes

  • UTF-8 validation in Write::write(): Validate bytes before writing, return InvalidData error for non-UTF8
  • Safety comment in to_str(): Document why internal write_to() calls cannot fail UTF-8 validation (ASCII-only writes)
  • Regression test: issue_2305_cannot_write_non_utf8_bytes verifies the PoC now fails correctly

The fix maintains the safety invariant that as_str() relies on while preserving all existing functionality (ASCII-only internal writes pass validation).

Original prompt

Fix #2305; Be sure to use the example in a test just to show that this isn't possible anymore with the fix.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix issue 2305 by adding test case fix: validate UTF-8 in TimeBuf::write() to prevent UB in as_str() Dec 28, 2025
Copilot AI requested a review from Byron December 28, 2025 12:20
It's a breaking change as the `std::io::Write` implementation was removed
from `TimeBuf`, which is only usable in `Time::to_str()` now.

Co-authored-by: Byron <63622+Byron@users.noreply.github.com>
@Byron Byron force-pushed the copilot/fix-issue-2305-test-example branch from f71f51c to f59a6cf Compare December 31, 2025 10:18
@Byron Byron marked this pull request as ready for review December 31, 2025 10:18
@Byron Byron requested a review from Copilot December 31, 2025 10:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a security vulnerability (issue #2305) where TimeBuf's public Write implementation allowed writing arbitrary non-UTF-8 bytes while as_str() used unsafe UTF-8 conversion, potentially causing undefined behavior.

Key Changes:

  • Removed the std::io::Write implementation from TimeBuf entirely to prevent external writes
  • Modified Time::to_str() to write directly to the internal SmallVec buffer, bypassing public API
  • Updated test to use Vec instead of TimeBuf for direct write testing

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
gix-date/src/parse/mod.rs Removed Write trait implementation for TimeBuf and updated to_str() to write directly to internal buffer
gix-date/tests/time/mod.rs Modified max() test to use Vec for testing write_to() independently from TimeBuf

- avoid a publicliy available `Write` impl in the first place.
@Byron Byron force-pushed the copilot/fix-issue-2305-test-example branch from f59a6cf to ef32312 Compare December 31, 2025 10:25
@Byron Byron enabled auto-merge December 31, 2025 10:26
@Byron Byron merged commit 76376ef into main Dec 31, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-utf8 String can be created with TimeBuf::as_str

2 participants