Skip to content
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

[C++] Not enough bytes are allocated for local buffer in StringFormatter<TimestampType> #41044

Closed
felipecrv opened this issue Apr 5, 2024 · 1 comment
Assignees
Milestone

Comments

@felipecrv
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

With a negative enough timestamp, a stack corruption (only one byte and only possible to force the write of a -) bug can be triggered.

     constexpr int64_t kMillisInDay = 24 * 60 * 60 * 1000;
     auto ty = timestamp(TimeUnit::MILLI, "+01:00");
     StringFormatter<TimestampType> formatter(ty.get());
     AssertFormatting(formatter, -15000 * 365 * kMillisInDay + 1,
                      "-13021-12-17 00:00:00.001Z");

On an ASAN build:

        [ RUN      ] Formatting.Timestamp
        =================================================================
        ==4191383==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff1804c48f at pc 0x5608edffe39d bp 0x7fff1804c110 sp 0x7fff1804c108
        WRITE of size 1 at 0x7fff1804c48f thread T0
            #0 0x5608edffe39c in arrow::internal::detail::FormatOneChar(char, char**) /home/felipeo/code/arrow/cpp/src/arrow/util/formatting.h:132:67
            #1 0x5608ee035c00 in arrow::internal::detail::FormatYYYY_MM_DD(arrow_vendored::date::year_month_day, char**) /home/felipeo/code/arrow/cpp/src/arrow/util/formatting.h:351:5
            #2 0x5608ee05e8a0 in decltype(std::declval<arrow::StringAppender&>()(std::basic_string_view<char, std::char_traits<char> >{})) arrow::internal::StringFormatter<arrow::TimestampType, void>::operator()<std::chrono::duration<long, std::ratio<1l, 1000l> >, arrow::StringAppender&>(std::chrono::duration<long, std::ratio<1l, 1000l> >, long, arrow::StringAppender&) /home/felipeo/code/arrow/cpp/src/arrow/util/form
atting.h
        :521:5
            #3 0x5608ee05d60f in decltype(std::declval<arrow::internal::StringFormatter<arrow::TimestampType, void>&>()(std::chrono::duration<long, std::ratio<1l, 1l> >{}, std::declval<long&>(), std::declval<arrow::StringAppender&>())) arrow::util::VisitDuration<arrow::internal::StringFormatter<arrow::TimestampType, void>&, long&, arrow::StringAppender&>(arrow::TimeUnit::type, arrow::internal::StringFormatter<arrow::
Timestam
        pType, void>&, long&, arrow::StringAppender&) /home/felipeo/code/arrow/cpp/src/arrow/util/time.h:60:14
            #4 0x5608ee05d122 in decltype(std::declval<arrow::StringAppender&>()(std::basic_string_view<char, std::char_traits<char> >{})) arrow::internal::StringFormatter<arrow::TimestampType, void>::operator()<arrow::StringAppender&>(long, arrow::StringAppender&) /home/felipeo/code/arrow/cpp/src/arrow/util/formatting.h:527:12
            #5 0x5608edfeffb3 in void arrow::AssertFormatting<arrow::internal::StringFormatter<arrow::TimestampType, void>, long>(arrow::internal::StringFormatter<arrow::TimestampType, void>&, long, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/felipeo/code/arrow/cpp/src/arrow/util/formatting_util_test.cc:52:3
            #6 0x5608edfece95 in arrow::Formatting_Timestamp_Test::TestBody() /home/felipeo/code/arrow/cpp/src/arrow/util/formatting_util_test.cc:540:5
            #7 0x7fd95d7901de in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/felipeo/code/arrow/cpp/ninja/debug/libarrow_testing.so.1600+0xd901de) (BuildId: dd9af0bafdb1786050262e8f6002568a9f08ecf6)
            #8 0x7fd95d784905 in testing::Test::Run() (/home/felipeo/code/arrow/cpp/ninja/debug/libarrow_testing.so.1600+0xd84905) (BuildId: dd9af0bafdb1786050262e8f6002568a9f08ecf6)
            #9 0x7fd95d784a84 in testing::TestInfo::Run() (/home/felipeo/code/arrow/cpp/ninja/debug/libarrow_testing.so.1600+0xd84a84) (BuildId: dd9af0bafdb1786050262e8f6002568a9f08ecf6)
            #10 0x7fd95d785038 in testing::TestSuite::Run() (/home/felipeo/code/arrow/cpp/ninja/debug/libarrow_testing.so.1600+0xd85038) (BuildId: dd9af0bafdb1786050262e8f6002568a9f08ecf6)
            #11 0x7fd95d78573e in testing::internal::UnitTestImpl::RunAllTests() (/home/felipeo/code/arrow/cpp/ninja/debug/libarrow_testing.so.1600+0xd8573e) (BuildId: dd9af0bafdb1786050262e8f6002568a9f08ecf6)
            #12 0x7fd95d7907a6 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/felipeo/code/arrow/cpp/ninja/debug/libarrow_testing.so.1600+0xd907a6) (BuildId: dd9af0bafdb1786050262e8f6002568a9f08ecf6)
            #13 0x7fd95d784b4b in testing::UnitTest::Run() (/home/felipeo/code/arrow/cpp/ninja/debug/libarrow_testing.so.1600+0xd84b4b) (BuildId: dd9af0bafdb1786050262e8f6002568a9f08ecf6)
            #14 0x5608ee54506d in RUN_ALL_TESTS() /usr/include/gtest/gtest.h:2490:46
            #15 0x5608ee544fb9 in main /home/felipeo/code/arrow/cpp/src/arrow/util/logging_test.cc:129:10
            #16 0x7fd93de29d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
            #17 0x7fd93de29e3f in __libc_start_main csu/../csu/libc-start.c:392:3
            #18 0x5608ed7b4f64 in _start (/home/felipeo/code/arrow/cpp/ninja/debug/arrow-utility-test+0x1224f64) (BuildId: 81cfdc36b7a960a7249ecd5884beaa869140ab89)

        Address 0x7fff1804c48f is located in stack of thread T0 at offset 399 in frame
            #0 0x5608ee05dc9f in decltype(std::declval<arrow::StringAppender&>()(std::basic_string_view<char, std::char_traits<char> >{})) arrow::internal::StringFormatter<arrow::TimestampType, void>::operator()<std::chrono::duration<long, std::ratio<1l, 1000l> >, arrow::StringAppender&>(std::chrono::duration<long, std::ratio<1l, 1000l> >, long, arrow::StringAppender&) /home/felipeo/code/arrow/cpp/src/arrow/util/form
atting.h
        :486

Component(s)

C++

felipecrv added a commit that referenced this issue Apr 6, 2024
…' when formatting timestamps (#41045)

### What changes are included in this PR?

A test that reproduces an issue found by the fuzzer and a fix for it.

### Are these changes tested?

 - A test
 - Comments clarifying somethings around `formatting.h`
 - Increasing the size of the local buffer used to format timestamps

The issue was introduced only recently (unreleased): #39272
* GitHub Issue: #41044

Authored-by: Felipe Oliveira Carvalho <[email protected]>
Signed-off-by: Felipe Oliveira Carvalho <[email protected]>
@felipecrv
Copy link
Contributor Author

Issue resolved by pull request 41045
#41045

@felipecrv felipecrv added this to the 16.0.0 milestone Apr 6, 2024
felipecrv added a commit to felipecrv/arrow that referenced this issue Apr 11, 2024
pitrou pushed a commit that referenced this issue Apr 11, 2024
)

### What changes are included in this PR?

Bump `arrow-testing` to include the latest commit.

Authored-by: Felipe Oliveira Carvalho <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this issue Apr 15, 2024
apache#41139)

### What changes are included in this PR?

Bump `arrow-testing` to include the latest commit.

Authored-by: Felipe Oliveira Carvalho <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
tolleybot pushed a commit to tmct/arrow that referenced this issue May 2, 2024
…the 'Z' when formatting timestamps (apache#41045)

### What changes are included in this PR?

A test that reproduces an issue found by the fuzzer and a fix for it.

### Are these changes tested?

 - A test
 - Comments clarifying somethings around `formatting.h`
 - Increasing the size of the local buffer used to format timestamps

The issue was introduced only recently (unreleased): apache#39272
* GitHub Issue: apache#41044

Authored-by: Felipe Oliveira Carvalho <[email protected]>
Signed-off-by: Felipe Oliveira Carvalho <[email protected]>
tolleybot pushed a commit to tmct/arrow that referenced this issue May 2, 2024
apache#41139)

### What changes are included in this PR?

Bump `arrow-testing` to include the latest commit.

Authored-by: Felipe Oliveira Carvalho <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
…the 'Z' when formatting timestamps (apache#41045)

### What changes are included in this PR?

A test that reproduces an issue found by the fuzzer and a fix for it.

### Are these changes tested?

 - A test
 - Comments clarifying somethings around `formatting.h`
 - Increasing the size of the local buffer used to format timestamps

The issue was introduced only recently (unreleased): apache#39272
* GitHub Issue: apache#41044

Authored-by: Felipe Oliveira Carvalho <[email protected]>
Signed-off-by: Felipe Oliveira Carvalho <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
apache#41139)

### What changes are included in this PR?

Bump `arrow-testing` to include the latest commit.

Authored-by: Felipe Oliveira Carvalho <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant