0001752: [memcheck] syscall param socketcall.sendto(msg) points to uninitialised byte(s)

View Issue Details [ Jump to Notes ]

[ Issue History ] [ Print ]

Project

Category

View Status

Date Submitted

Last Update

0001752

aMule

Misc

public

2015-07-12 14:15

2015-11-06 18:21

Reporter

sirkay2006

Assigned To

Priority

high

Severity

minor

Reproducibility

always

Status

new

Resolution

open

Platform

linux amd_64 bit

linux debian

OS Version

Product Version

SVN

Target Version

2.3.2

Fixed in Version

Summary

0001752: [memcheck] syscall param socketcall.sendto(msg) points to uninitialised byte(s)

Description

When running aMule on valgrind, the following syscall param socketcall.sendto(msg) points to uninitialised byte(s).

==6205== Thread 4:
==6205== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==6205== at 0x4E41013: ??? (syscall-template.S:81)
==6205== by 0x6258843: GSocket::Send_Dgram(char const*, int) (in /home/ttt/_bin/bin/wxWidgets-2.8.12/lib/libwx_baseu_net-2.8.so.0.8.0)
==6205== by 0x62588CF: GSocket::Write(char const*, int) (in /home/ttt/_bin/bin/wxWidgets-2.8.12/lib/libwx_baseu_net-2.8.so.0.8.0)
==6205== by 0x62544E7: wxSocketBase::_Write(void const*, unsigned int) (in /home/ttt/_bin/bin/wxWidgets-2.8.12/lib/libwx_baseu_net-2.8.so.0.8.0)
==6205== by 0x6254523: wxSocketBase::Write(void const*, unsigned int) (in /home/ttt/_bin/bin/wxWidgets-2.8.12/lib/libwx_baseu_net-2.8.so.0.8.0)
==6205== by 0x6255157: wxDatagramSocket::SendTo(wxSockAddress const&, void const*, unsigned int) (in /home/ttt/_bin/bin/wxWidgets-2.8.12/lib/libwx_baseu_net-2.8.so.0.8.0)
==6205== by 0x5D2045: CLibUDPSocket::SendTo(amuleIPV4Address const&, void const*, unsigned int) (LibSocketWX.cpp:78)
==6205== by 0x59A790: CDatagramSocketProxy::SendTo(amuleIPV4Address const&, void const*, unsigned int) (Proxy.cpp:1458)
==6205== by 0x4C4BF9: CMuleUDPSocket::SendTo(unsigned char*, unsigned int, unsigned int, unsigned short) (MuleUDPSocket.cpp:316)
==6205== by 0x4C4FA1: CMuleUDPSocket::SendControlData(unsigned int, unsigned int) (MuleUDPSocket.cpp:277)
==6205== by 0x4F0ECA: UploadBandwidthThrottler::Entry() (UploadBandwidthThrottler.cpp:381)
==6205== by 0x6550F64: wxThreadInternal::PthreadStart(wxThread*) (in /home/ttt/_bin/bin/wxWidgets-2.8.12/lib/libwx_baseu-2.8.so.0.8.0)
==6205== by 0x4E3A0A3: start_thread (pthread_create.c:309)
==6205== by 0x70CF04C: clone (clone.S:111)
==6205== Address 0x1baf1fdf is 31 bytes inside a block of size 51 alloc'd
==6205== at 0x4C2873C: operator new[](unsigned long) (vg_replace_malloc.c:389)
==6205== by 0x4A070E: CEncryptedDatagramSocket::EncryptSendClient(unsigned char**, int, unsigned char const*, bool, unsigned int, unsigned int) (EncryptedDatagramSocket.cpp:274)
==6205== by 0x4C50B3: CMuleUDPSocket::SendControlData(unsigned int, unsigned int) (MuleUDPSocket.cpp:274)
==6205== by 0x4F0ECA: UploadBandwidthThrottler::Entry() (UploadBandwidthThrottler.cpp:381)
==6205== by 0x6550F64: wxThreadInternal::PthreadStart(wxThread*) (in /home/ttt/_bin/bin/wxWidgets-2.8.12/lib/libwx_baseu-2.8.so.0.8.0)
==6205== by 0x4E3A0A3: start_thread (pthread_create.c:309)
==6205== by 0x70CF04C: clone (clone.S:111)
==6205== Uninitialised value was created by a stack allocation
==6205== at 0x51B0B0: Kademlia::CRoutingZone::RandomLookup() const (RoutingZone.cpp:820)

I do not know the code well enough to see the bug underlying this report (why cryptedBuffer is not set in EncryptSendClient, but sent later on).

I suggest to procted the code by adding some initialisation of the buffer, like:
diff --git a/src/EncryptedDatagramSocket.cpp b/src/EncryptedDatagramSocket.cpp
index aa898c3..e0bb912 100644
--- a/src/EncryptedDatagramSocket.cpp
+++ b/src/EncryptedDatagramSocket.cpp
@@ -272,6 +272,7 @@ int CEncryptedDatagramSocket::EncryptSendClient(uint8_t **buf, int bufLen, const
        const uint32_t cryptHeaderLen = padLen + CRYPT_HEADER_WITHOUTPADDING + (kad ? 8 : 0);
        uint32_t cryptedLen = bufLen + cryptHeaderLen;
        uint8_t *cryptedBuffer = new uint8_t[cryptedLen];
+ memset(cryptedBuffer, 0, sizeof(uint8_t) * cryptedLen);
        bool kadRecvKeyUsed = false;

        uint16_t randomKeyPart = GetRandomUint16();
diff --git a/src/EncryptedDatagramSocket.cpp b/src/EncryptedDatagramSocket.cpp
index aa898c3..e0bb912 100644
--- a/src/EncryptedDatagramSocket.cpp
+++ b/src/EncryptedDatagramSocket.cpp
@@ -272,6 +272,7 @@ int CEncryptedDatagramSocket::EncryptSendClient(uint8_t **buf, int bufLen, const
        const uint32_t cryptHeaderLen = padLen + CRYPT_HEADER_WITHOUTPADDING + (kad ? 8 : 0);
        uint32_t cryptedLen = bufLen + cryptHeaderLen;
        uint8_t *cryptedBuffer = new uint8_t[cryptedLen];
+ memset(cryptedBuffer, 0, sizeof(uint8_t) * cryptedLen);
        bool kadRecvKeyUsed = false;

        uint16_t randomKeyPart = GetRandomUint16();

Do you mind investigating?

Thanks!

Notes
(0003660) sirkay2006 (reporter) 2015-07-12 14:49	Unfortunately memset does not correct the issue. :-(

(0003667) sirkay2006 (reporter) 2015-11-06 18:21	Problem still occurring with g0023527

Issue History
Date Modified	Username	Field	Change
2015-07-12 14:15	sirkay2006	New Issue
2015-07-12 14:49	sirkay2006	Note Added: 0003660
2015-10-17 23:17	GonoszTopi	Target Version	=> 2.3.2
2015-11-06 18:21	sirkay2006	Note Added: 0003667

Relationships