You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2020/02/27 11:27:53 UTC
[GitHub] [rocketmq-client-cpp] WoodsCumming opened a new pull request #264:
Fix a heap-buffer-overflow risk due to wrong use of string constructor.
WoodsCumming opened a new pull request #264: Fix a heap-buffer-overflow risk due to wrong use of string constructor.
URL: https://github.com/apache/rocketmq-client-cpp/pull/264
## What is the purpose of the change
There is a heap-buffer-overflow risk that when the chars is not with '\0' or chars contain binary data, also may be a short parse. The string constructor should be replace with 'basic_string( const CharT* s, size_type count, const Allocator& alloc = Allocator() )' that specified chars length need be allocate other than expect the constructor 'basic_string( const CharT* s, const Allocator& alloc = Allocator() )' to find '\0' for calculating chars length.
## Brief changelog
Fix a heap-buffer-overflow risk due to wrong use of string constructor.
## Verifying this change
Has verified. Want a code review.
## The ASAN Report
==101002==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60700000bd6e at pc 0x49a848 bp 0x7ffe7ae80050 sp 0x7ffe7ae80028
READ of size 79 at 0x60700000bd6e thread T0
#0 0x49a847 in strlen (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x49a847)
#1 0x7fc253934ef0 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (/usr/local/gcc-4.9.2/lib64/libstdc++.so.6+0xbfef0)
#2 0x7fc2541ad1e7 in rocketmq::LockBatchResponseBody::Decode(rocketmq::MemoryBlock const*, std::vector<rocketmq::MQMessageQueue, std::allocator<rocketmq::MQMessageQueue> >&) (/home/yizhe.wcm/PR/rocketmq-client-cpp/bin/librocketmq.so+0x62e1e7)
#3 0x4e809c in lockBatchBody_LockBatchResponseBody_Test::TestBody() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4e809c)
#4 0x51ea35 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x51ea35)
#5 0x518789 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x518789)
#6 0x4f940a in testing::Test::Run() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4f940a)
#7 0x4f9cd1 in testing::TestInfo::Run() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4f9cd1)
#8 0x4fa395 in testing::TestCase::Run() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4fa395)
#9 0x504b98 in testing::internal::UnitTestImpl::RunAllTests() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x504b98)
#10 0x51fe33 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x51fe33)
#11 0x51950b in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x51950b)
#12 0x50363e in testing::UnitTest::Run() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x50363e)
#13 0x4e9a23 in RUN_ALL_TESTS() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4e9a23)
#14 0x4e85be in main (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4e85be)
#15 0x7fc252fb0444 in __libc_start_main (/lib64/libc.so.6+0x22444)
#16 0x482c88 (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x482c88)
0x60700000bd6e is located 0 bytes to the right of 78-byte region [0x60700000bd20,0x60700000bd6e)
allocated by thread T0 here:
#0 0x4bda1f in malloc (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4bda1f)
#1 0x7fc2540bd98f in rocketmq::MemoryBlock::MemoryBlock(void const*, unsigned long) (/home/yizhe.wcm/PR/rocketmq-client-cpp/bin/librocketmq.so+0x53e98f)
#2 0x4e8061 in lockBatchBody_LockBatchResponseBody_Test::TestBody() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4e8061)
#3 0x51ea35 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x51ea35)
#4 0x518789 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x518789)
#5 0x4f940a in testing::Test::Run() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4f940a)
#6 0x4f9cd1 in testing::TestInfo::Run() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4f9cd1)
#7 0x4fa395 in testing::TestCase::Run() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4fa395)
#8 0x504b98 in testing::internal::UnitTestImpl::RunAllTests() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x504b98)
#9 0x51fe33 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x51fe33)
#10 0x51950b in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x51950b)
#11 0x50363e in testing::UnitTest::Run() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x50363e)
#12 0x4e9a23 in RUN_ALL_TESTS() (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4e9a23)
#13 0x4e85be in main (/home/yizhe.wcm/PR/rocketmq-client-cpp/test/bin/LockBatchBodyTest+0x4e85be)
#14 0x7fc252fb0444 in __libc_start_main (/lib64/libc.so.6+0x22444)
SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 strlen
Shadow bytes around the buggy address:
0x0c0e7fff9750: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e7fff9760: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e7fff9770: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e7fff9780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e7fff9790: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c0e7fff97a0: fa fa fa fa 00 00 00 00 00 00 00 00 00[06]fa fa
0x0c0e7fff97b0: fa fa 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
0x0c0e7fff97c0: 00 00 00 00 00 00 00 00 00 fa fa fa fa fa 00 00
0x0c0e7fff97d0: 00 00 00 00 00 00 00 fa fa fa fa fa 00 00 00 00
0x0c0e7fff97e0: 00 00 00 00 00 fa fa fa fa fa 00 00 00 00 00 00
0x0c0e7fff97f0: 00 00 00 fa fa fa fa fa 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Contiguous container OOB:fc
ASan internal: fe
==101002==ABORTING
Follow this checklist to help us incorporate your contribution quickly and easily. Notice, `it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR`.
- [x] Make sure there is a [Github issue](https://github.com/apache/rocketmq/issues) filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [x] Format the pull request title like `[ISSUE #123] Fix UnknownException when host config not exist`. Each commit in the pull request should have a meaningful subject line and body.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when a cross-module dependency exists.
- [ ] If this contribution is large, please file an [Apache Individual Contributor License Agreement](http://www.apache.org/licenses/#clas).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq-client-cpp] ShannonDing merged pull request #264:
fix(memery): heap-buffer-overflow risk due to wrong use of string
constructor.
Posted by GitBox <gi...@apache.org>.
ShannonDing merged pull request #264: fix(memery): heap-buffer-overflow risk due to wrong use of string constructor.
URL: https://github.com/apache/rocketmq-client-cpp/pull/264
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq-client-cpp] codecov-io commented on issue #264: Fix a
heap-buffer-overflow risk due to wrong use of string constructor.
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #264: Fix a heap-buffer-overflow risk due to wrong use of string constructor.
URL: https://github.com/apache/rocketmq-client-cpp/pull/264#issuecomment-591929139
# [Codecov](https://codecov.io/gh/apache/rocketmq-client-cpp/pull/264?src=pr&el=h1) Report
> Merging [#264](https://codecov.io/gh/apache/rocketmq-client-cpp/pull/264?src=pr&el=desc) into [master](https://codecov.io/gh/apache/rocketmq-client-cpp/commit/12a73b1f0c750c9582c9f816dd0d11218728ddd8?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/rocketmq-client-cpp/pull/264?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #264 +/- ##
=======================================
Coverage 58.28% 58.28%
=======================================
Files 182 182
Lines 11805 11805
=======================================
Hits 6880 6880
Misses 4925 4925
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq-client-cpp/pull/264?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/protocol/LockBatchBody.cpp](https://codecov.io/gh/apache/rocketmq-client-cpp/pull/264/diff?src=pr&el=tree#diff-c3JjL3Byb3RvY29sL0xvY2tCYXRjaEJvZHkuY3Bw) | `93.42% <100%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq-client-cpp/pull/264?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq-client-cpp/pull/264?src=pr&el=footer). Last update [12a73b1...a647e06](https://codecov.io/gh/apache/rocketmq-client-cpp/pull/264?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services