Receive the whole IP string to avoid the truncation of the last digit

[Skeptor]

Related issue
#3511

Description

When the agent IP had the maximum length, 15, the last digit was truncated because it was being replaced with \0. This PR changes the number of characters received to receive the \0 from the socket too and avoid the truncation.

The field agent_ip of the agent-info was truncated before:

After the change, the IP field contains the whole IP

Tests

• Compile agent on Linux
• Source installation
• Agent with IP XXX.XXX.XXX.XXX

Memory Test of ossec-agentd

[chemamartinez]

I am missing the memory tests.

[chemamartinez]

I've found an invalid write when applying this change. This is why we are allocating a size of 16 for agent_ip here:

wazuh/src/client-agent/notify.c

Line 138 in 170da17

os_calloc(16,sizeof(char),agent_ip);

If we set the same size when calling OS_RecvUnix() here (IPSIZE = 16):

wazuh/src/client-agent/notify.c

Line 146 in 170da17

if(OS_RecvUnix(sock, IPSIZE, agent_ip) == 0){

At OS_RecvUnix() it is written the position 17 of agent_ip which is out of bounds:

wazuh/src/os_net/os_net.c

Line 470 in 170da17

ret[sizet] = '\0';

That's causing the following invalid write:

==6157== Invalid write of size 1
==6157==    at 0x1767D1: OS_RecvUnix (os_net.c:470)
==6157==    by 0x117845: run_notify (notify.c:146)
==6157==    by 0x112349: AgentdStart (agentd.c:200)
==6157==    by 0x114E6B: main (main.c:179)
==6157==  Address 0x62c7580 is 0 bytes after a block of size 16 alloc'd
==6157==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==6157==    by 0x117746: run_notify (notify.c:138)
==6157==    by 0x112349: AgentdStart (agentd.c:200)
==6157==    by 0x114E6B: main (main.c:179)

[chemamartinez]

Comments at the PR conversation.

[chemamartinez]

More desired changes:

1. Allocate the memory relative to IPSIZE instead of hardcoding the 16:

   wazuh/src/client-agent/notify.c

   Line 138 in 170da17

   os_calloc(16,sizeof(char),agent_ip);

2. The buffer allocated and used here:

   wazuh/src/wazuh_modules/wm_control.c

   Line 201 in 170da17

   switch (length = OS_RecvUnix(peer, IPSIZE - 1, buffer), length) {

   
   The received content of this buffer is no longer used, in addition we are passing IPSIZE - 1 here too. Review it please.

[vikman90]

LGTM!

OS_RecvUnix(int socket, char *buffer, int buffer_size) reads up to buffer_size - 1 bytes, and it terminates the string.

Edit

OS_RecvUnix(int socket, char *buffer, int buffer_size)

• First terminates the string, assuming that buffer allocates buffer_size + 1 bytes.
• Reads up to buffer_size - 1.
• Terminates the resulting string.

So, if an IP occupies IPSIZE - 1 bytes, we must allocate IPSIZE + 1 bytes and pass IPSIZE to OS_RecvUnix():

char buffer[IPSIZE + 1];
OS_RecvUnix(sock, buffer, IPSIZE);

This way, buffer will be terminated twice.

[vikman90]

LGTM!

[bah07]

LGTM!