Memory issues observed while running protocol_handler_test

[shoamano83]

Bug Report

Several memory issues such as buffer over-run, mismatched malloc / delete[], accessing to uninitialized memory and memory leak are observed while running protocol_handler_test with valgrind.
Some issues are because of incorrect implementation of unit tests, while a few are because of Core implementation issues.

Reproduction Steps

1. Check out develop branch (commit 7efbe41)
2. Turn on BUILD_TESTS in CMakeLists.txt and give a build
3. Run protocol_handler_test unit test with valgrind, e.g.:

$ cd src/components/protocol_handler/test/
$ valgrind --trace-children=yes --leak-check=full ./protocol_handler_test > protocol_handler_test_valgrind.txt 2>&1

Expected Behavior

Memory issues should not be observed.

Observed Behavior

Valgrind reports several memory issues.
Note that the ones with "unit test issue" label are because of incorrect implementation of unit tests. They are not Core issue.

(1) Buffer over-run in ProtocolPayloadTest.ExtractProtocolWithJSONWithDataWithWrongPayloadSize (unit test issue)

[ RUN      ] ProtocolPayloadTest.ExtractProtocolWithJSONWithDataWithWrongPayloadSize
==18478== Invalid write of size 1
==18478==    at 0x73610C: test::components::protocol_handler_test::prepare_data(unsigned char*, protocol_handler::ProtocolPayloadV2&) (protocol_payload_test.cc:80)

(2) Mismatched malloc and delete[] in ProtocolHandlerImpl::NotifySessionStarted(), occurs when ENABLE_SECURITY is enabled

[ RUN      ] ProtocolHandlerImplTest.SecurityEnable_StartSessionProtected_Fail
==18478== Thread 2:
==18478== Mismatched free() / delete / delete []
==18478==    at 0x4C2F650: operator delete[](void*) (vg_replace_malloc.c:621)
==18478==    by 0x8A0BDC: protocol_handler::ProtocolHandlerImpl::NotifySessionStarted(protocol_handler::SessionContext const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&)::{lambda(unsigned char*)#2}::operator()(unsigned char*) const (protocol_handler_impl.cc:1557)

(3) accessing to uninitialized memory in two tests (unit test issue)

[ RUN      ] ProtocolHandlerImplTest.SendEndServicePrivate_ServiceTypeControl_MessageSent
==18478== Thread 3:
==18478== Conditional jump or move depends on uninitialised value(s)
==18478==    at 0x88C90F: protocol_handler::ProtocolPacket::serializePacket() const (protocol_packet.cc:449)

[ RUN      ] IncomingDataHandlerTest.MalformedPacket_Mix
==18478== Conditional jump or move depends on uninitialised value(s)
==18478==    at 0x888FC4: protocol_handler::ProtocolPacket::ProtocolHeader::deserialize(unsigned char const*, unsigned long) (protocol_packet.cc:156)

(4) accessing to uninitialized memory area while comparing buffers (unit test issue)

[ RUN      ] IncomingDataHandlerTest.MixedPayloadData_TwoConnections
==18478== Thread 1:
==18478== Conditional jump or move depends on uninitialised value(s)
==18478==    at 0x4C337DD: __memcmp_sse4_1 (vg_replace_strmem.c:1099)
==18478==    by 0x88CDD7: protocol_handler::ProtocolPacket::operator==(protocol_handler::ProtocolPacket const&) const (protocol_packet.cc:514)

(5) memory leak in ProtocolHandlerImpl::NotifySessionStarted()

==18478== 40 bytes in 1 blocks are definitely lost in loss record 2 of 8
==18478==    at 0x4C2E216: operator new(unsigned long) (vg_replace_malloc.c:334)
==18478==    by 0x8A11FF: protocol_handler::ProtocolHandlerImpl::NotifySessionStarted(protocol_handler::SessionContext const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&) (protocol_handler_impl.cc:1494)

(6) another memory leak in ProtocolHandlerImpl::NotifySessionStarted(), occurs when ENABLE_SECURITY is enabled

==18478== 372 (256 direct, 116 indirect) bytes in 4 blocks are definitely lost in loss record 6 of 8
==18478==    at 0x4C2E216: operator new(unsigned long) (vg_replace_malloc.c:334)
==18478==    by 0x8A230F: protocol_handler::ProtocolHandlerImpl::NotifySessionStarted(protocol_handler::SessionContext const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&) (protocol_handler_impl.cc:1633)

OS & Version Information

• OS/Version: Ubuntu 16.04 (amd64)
• SDL Core Version: commit 7efbe41 from develop branch
• Testing Against: N/A (unit testing)

Test Case, Sample Code, and / or Example App

Output from valgrind is attached: protocol_handler_test_valgrind.txt

[iCollin]

Closed via #2270