Horrible memory leak in recvfrom() that has gone completely unnoticed until now?

[pmt]

I'm using the latest MQX Classic release, 4.2.0.2 and we're having a receive buffer overflow issue in recvfrom().  Hopefully the MQX developers are still active on this board.  

Shouldn't this line in the code snippet below:

   memcpy(parms->udpptr, dgram_item->dgram, dgram_size);

be this instead:

   memcpy(parms->udpptr, dgram_item->dgram, parms->udpword); 

Otherwise you can overflow the user receive buffer causing a memory leak (you ignore the buffer length parameter).

Thanks,

PMT

static void udp_return_req2socket_from_rx_queue(UDP_PARM_PTR parms, struct udp_rx_dgram_header * dgram_item)
{
uint32_t dgram_size = 0;

dgram_size = dgram_item->size;

/* limit for the upper layer recv buffer */
if(parms->udpword > dgram_size)
{
parms->udpword = dgram_size;
}

if(parms->saddr_ptr)
{
*parms->saddr_ptr = dgram_item->fromaddr;
}
memcpy(parms->udpptr, dgram_item->dgram, dgram_size);
((SOCKET_STRUCT_PTR)parms->ucb->SOCKET)->LINK_OPTIONS.RX = dgram_item->rx_linkopts;
}

static void udp_return_req2socket_from_pcb(UDP_PARM_PTR parms, RTCSPCB_PTR pcb_ptr)
{
uint32_t dgram_size = 0;

dgram_size = RTCSPCB_SIZE(pcb_ptr);

/* limit for the upper layer recv buffer */
if(parms->udpword > dgram_size)
{
parms->udpword = dgram_size;
}

if(parms->saddr_ptr)
{
uint16_t af = ((SOCKET_STRUCT_PTR)parms->ucb->SOCKET)->AF;
udp_set_fromaddr(parms->saddr_ptr, pcb_ptr, af);
}
RTCSPCB_memcopy(pcb_ptr, parms->udpptr, 0, parms->udpword);
((SOCKET_STRUCT_PTR)parms->ucb->SOCKET)->LINK_OPTIONS.RX = pcb_ptr->LINK_OPTIONS.RX;
}

[craig_honegger_]

pmt;

Thanks for your feedback. Your fix is correct. In MQX v5, we are changing all memcpy and _mem_copy  calls to _mem_copy_s which requires both the source and destination buffer lengths to be provided.

Craig

[danielchen]

Hi pmt 

Thank you very much for your feedback. This seems an issue happened by receive longer UDP datagram.  Your solution can fix that. Since Classic MQX 4.2 now moved to MQX v5, it will be fixed in next MQX v5.

Regards

Daniel