Skip to content

WebSocketTest.cpp faults reported by valgrind #2323

@roccocorsi

Description

@roccocorsi

Ran valgrind to verify NetSSL_OpenSSL testrunner and found two problems. Similar issues also present on Net and NetSSL_Win.

Found following issues:

  1. Wrong 'delete' on unique_ptr could lead to memory corruption in testrunner. Applies to NetSSL_OpenSSL and NetSSL_Win, but does not to Net.

==7913== Thread 2:
==7913== Mismatched free() / delete / delete []
==7913== at 0x4C2B8DC: operator delete(void*) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==7913== by 0x436AC3: std::default_delete::operator()(char*) const (unique_ptr.h:67)
==7913== by 0x4368EC: std::unique_ptr<char, std::default_delete >::~unique_ptr() (unique_ptr.h:184)
==7913== by 0x433CA2: (anonymous namespace)::WebSocketRequestHandler::handleRequest(Poco::Net::HTTPServerRequest&, Poco::Net::HTTPServerResponse&) (WebSocketTest.cpp:52)
==7913== by 0x51F3303: Poco::Net::HTTPServerConnection::run() (HTTPServerConnection.cpp:85)
==7913== by 0x521562B: Poco::Net::TCPServerConnection::start() (TCPServerConnection.cpp:43)
==7913== by 0x522D338: Poco::Net::TCPServerDispatcher::run() (TCPServerDispatcher.cpp:114)
==7913== by 0x62987A6: Poco::PooledThread::run() (ThreadPool.cpp:214)
==7913== by 0x6294086: Poco::(anonymous namespace)::RunnableHolder::run() (Thread.cpp:43)
==7913== by 0x6293ECC: Poco::ThreadImpl::runnableEntry(void*) (Thread_STD.cpp:139)
==7913== by 0x6296D6B: void* std::_Bind_simple<void* ((Poco::ThreadImpl))(void*)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) (functional:1732)
==7913== by 0x6296C76: std::_Bind_simple<void* ((Poco::ThreadImpl))(void*)>::operator()() (functional:1720)
==7913== Address 0xbf1e5f0 is 0 bytes inside a block of size 1,024 alloc'd
==7913== at 0x4C2AEA0: operator new[](unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==7913== by 0x433C1D: (anonymous namespace)::WebSocketRequestHandler::handleRequest(Poco::Net::HTTPServerRequest&, Poco::Net::HTTPServerResponse&) (WebSocketTest.cpp:52)
==7913== by 0x51F3303: Poco::Net::HTTPServerConnection::run() (HTTPServerConnection.cpp:85)
==7913== by 0x521562B: Poco::Net::TCPServerConnection::start() (TCPServerConnection.cpp:43)
==7913== by 0x522D338: Poco::Net::TCPServerDispatcher::run() (TCPServerDispatcher.cpp:114)
==7913== by 0x62987A6: Poco::PooledThread::run() (ThreadPool.cpp:214)
==7913== by 0x6294086: Poco::(anonymous namespace)::RunnableHolder::run() (Thread.cpp:43)
==7913== by 0x6293ECC: Poco::ThreadImpl::runnableEntry(void*) (Thread_STD.cpp:139)
==7913== by 0x6296D6B: void* std::_Bind_simple<void* ((Poco::ThreadImpl))(void*)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) (functional:1732)
==7913== by 0x6296C76: std::_Bind_simple<void* ((Poco::ThreadImpl))(void*)>::operator()() (functional:1720)
==7913== by 0x6296C0F: std::thread::_Impl<std::_Bind_simple<void* ((Poco::ThreadImpl))(void*)> >::_M_run() (thread:115)
==7913== by 0x77BBA1F: ??? (in /usr/lib64/libstdc++.so.6.0.24)

Problem is on WebSocketTest.cpp:52 for NetSSL_OpenSSL and in same location for NetSSL_Win. Problem not found in Net package.

std::unique_ptr pBuffer(new char[_bufSize]);

'char' to be changed to 'char[]'

std::unique_ptr<char[]> pBuffer(new char[_bufSize]);

  1. Uninitialized 'buffer' reported by valgrind when converted to std::string in 5 parm string compare(). Could lead to segfault if invalid memory accessed.

==12969== Conditional jump or move depends on uninitialised value(s)
==12969== at 0x4C2D4B8: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==12969== by 0x77D33A4: std::basic_string<char, std::char_traits, std::allocator >::basic_string(char const*, std::allocator const&) (in /usr/lib64/libstdc++.so.6.0.24)
==12969== by 0x434383: WebSocketTest::testWebSocket() (WebSocketTest.cpp:133)
==12969== by 0x437006: CppUnit::TestCaller::runTest() (TestCaller.h:75)
==12969== by 0x65EA334: CppUnit::TestCase::run(CppUnit::TestResult*) (TestCase.cpp:116)
==12969== by 0x65E967F: CppUnit::TestSuite::run(CppUnit::TestResult*) (TestSuite.cpp:30)
==12969== by 0x65E967F: CppUnit::TestSuite::run(CppUnit::TestResult*) (TestSuite.cpp:30)
==12969== by 0x65E967F: CppUnit::TestSuite::run(CppUnit::TestResult*) (TestSuite.cpp:30)
==12969== by 0x65EBDAA: CppUnit::TestRunner::run(CppUnit::Test*) (TestRunner.cpp:133)
==12969== by 0x65EBC57: CppUnit::TestRunner::run(std::vector<std::string, std::allocatorstd::string > const&) (TestRunner.cpp:108)
==12969== by 0x412547: NetSSLApp::main(std::vector<std::string, std::allocatorstd::string > const&) (Driver.cpp:41)
==12969== by 0x57DD41C: Poco::Util::Application::run() (Application.cpp:335)

issue is on:
(WebSocketTest.cpp:133)
(WebSocketTest.cpp:142)
(WebSocketTest.cpp:152)
(WebSocketTest.cpp:160)
(WebSocketTest.cpp:167)
(WebSocketTest.cpp:209)

Problem is with the 'compare':
payload.compare(0, payload.size(), buffer, 0, n) == 0

int compare (size_t pos, size_t len, const string& str,
size_t subpos, size_t sublen) const;

variable 'buffer' in parm3 must be converted to std::string, but it may not be properly initialized and terminated with NULL leading valgrind to complain that uninitialized memory accessed.

Fixed by zeroing out buffer, but changing to 4 parm string compare without std::string conversion would be better.

Better compare() would be this one that doesn't use std::string for buffer.

int compare (size_t pos, size_t len, const char* s, size_t n) const;

Zeroing out buffer is ok solution.

Metadata

Metadata

Labels

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions