-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[libc] Fix off by one error in strftime #165711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-libc Author: Marcell Leleszi (mleleszi) ChangesThis patch fixes a bug in strftime's return value when the formatted output exactly fills the buffer, not including the null terminator. The previous check failed to account for the null terminator in this case, incorrectly returning the written count instead of 0. Full diff: https://github.com/llvm/llvm-project/pull/165711.diff 3 Files Affected:
diff --git a/libc/src/time/strftime.cpp b/libc/src/time/strftime.cpp
index f36091bc9736e..89b7d9bb7c1b9 100644
--- a/libc/src/time/strftime.cpp
+++ b/libc/src/time/strftime.cpp
@@ -26,7 +26,7 @@ LLVM_LIBC_FUNCTION(size_t, strftime,
int ret = strftime_core::strftime_main(&writer, format, timeptr);
if (buffsz > 0) // if the buffsz is 0 the buffer may be a null pointer.
wb.buff[wb.buff_cur] = '\0';
- return (ret < 0 || static_cast<size_t>(ret) > buffsz) ? 0 : ret;
+ return (ret < 0 || static_cast<size_t>(ret) >= buffsz) ? 0 : ret;
}
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/time/strftime_l.cpp b/libc/src/time/strftime_l.cpp
index 201b85da39ee2..409f8683b7289 100644
--- a/libc/src/time/strftime_l.cpp
+++ b/libc/src/time/strftime_l.cpp
@@ -29,7 +29,7 @@ LLVM_LIBC_FUNCTION(size_t, strftime_l,
int ret = strftime_core::strftime_main(&writer, format, timeptr);
if (buffsz > 0) // if the buffsz is 0 the buffer may be a null pointer.
wb.buff[wb.buff_cur] = '\0';
- return (ret < 0 || static_cast<size_t>(ret) > buffsz) ? 0 : ret;
+ return (ret < 0 || static_cast<size_t>(ret) >= buffsz) ? 0 : ret;
}
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/test/src/time/strftime_test.cpp b/libc/test/src/time/strftime_test.cpp
index cac7560b2b945..38176f77804d5 100644
--- a/libc/test/src/time/strftime_test.cpp
+++ b/libc/test/src/time/strftime_test.cpp
@@ -2326,3 +2326,23 @@ TEST(LlvmLibcStrftimeTest, TimeFormatFullDateTime) {
// size_t written = 0;
// SimplePaddedNum spn;
// }
+
+TEST(LlvmLibcStrftimeTest, BufferTooSmall) {
+ struct tm time;
+ char buffer[1];
+
+ time.tm_year = get_adjusted_year(2025);
+ time.tm_mon = 10;
+ time.tm_mday = 24;
+
+ size_t written =
+ LIBC_NAMESPACE::strftime(buffer, sizeof(buffer), "%F", &time);
+ EXPECT_EQ(written, size_t{0});
+
+ char buffer2[10];
+
+ // The string "2025-11-24" is 10 chars,
+ // so strftime needs 10 + 1 bytes to write the string and the null terminator.
+ written = LIBC_NAMESPACE::strftime(buffer, sizeof(buffer2), "%F", &time);
+ EXPECT_EQ(written, size_t{0});
+}
|
|
@michaelrj-google Can you please take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the cleanup!
|
Can you merge please? I don't have access yet |
A typo in llvm#165711 caused sanitizer failures (the small buffer was used for the larger test). Renamed the variables to avoid the mistake in future.
A typo in #165711 caused sanitizer failures (the small buffer was used for the larger test). Renamed the variables to avoid the mistake in future.
This patch fixes a bug in strftime's return value when the formatted output exactly fills the buffer, not including the null terminator. The previous check failed to account for the null terminator in this case, incorrectly returning the written count instead of 0.
A typo in llvm#165711 caused sanitizer failures (the small buffer was used for the larger test). Renamed the variables to avoid the mistake in future.
This patch fixes a bug in strftime's return value when the formatted output exactly fills the buffer, not including the null terminator. The previous check failed to account for the null terminator in this case, incorrectly returning the written count instead of 0.
A typo in llvm#165711 caused sanitizer failures (the small buffer was used for the larger test). Renamed the variables to avoid the mistake in future.
This patch fixes a bug in strftime's return value when the formatted output exactly fills the buffer, not including the null terminator. The previous check failed to account for the null terminator in this case, incorrectly returning the written count instead of 0.