0

I got the following two functions for sending and receiving strings in C. When I use sendStrBuffer() in my client maching and recvStrBuffer() in my server, it works fine. But when I try to use the sendStrBuffer() on server and recvStrBuffer() in client, it won't work. It throws me an error code: 10014 which means:

Bad address. The system detected an invalid pointer address in attempting to use a pointer argument of a call. This error occurs if an application passes an invalid pointer value, or if the length of the buffer is too small. For instance, if the length of an argument, which is a sockaddr structure, is smaller than the sizeof(sockaddr).

When I don't use these function but the normal send() and recv() (wihtout the while loops), I don't get any errors. So I'm doing something wrong in my one of my functions (probably buffer overflow) but can't point at it.

int sendStrBuffer(char* buffer, int sizeBuffer)
{
    // send buffer size
    uint32_t num = htonl(sizeBuffer);
    char* converted_num = (char*)#
    int res = send(ClientSocket, converted_num, sizeof(uint32_t), 0);
    if (res == SOCKET_ERROR)
    {
        printf("error send\n");
    }

    // send buffer
    int totalSent = 0;
    int sent = 0;

    while (totalSent < sizeBuffer)
    {
        sent = send(ClientSocket, buffer + totalSent, sizeBuffer - totalSent, 0);
        if (sent == SOCKET_ERROR)
        {
            printf("error sending buffer\n");
            return SOCKET_ERROR;
        }
        totalSent += sent;
        //printf("sent: %d\n", sent);
        //printf("totalSent: %d\n", totalSent);
    }
}

int recvStrBuffer(SOCKET s, char* buffer, int bzeroSize)
{
    bzero(buffer, bzeroSize);
    int totalReceived = 0;
    int received = 0;

    // recv buffer size
    char b[sizeof(uint32_t)];
    int r = recv(s, b, sizeof(uint32_t), 0);
    if (r == SOCKET_ERROR)
    {
        printf("error recv\n");
    }
    uint32_t sizeBuffer = ntohl_ch(&b[0]);

    // recv buffer
    while (totalReceived < sizeBuffer)
    {
        received = recv(s, buffer + totalReceived, sizeBuffer - totalReceived, 0);
        if (received == SOCKET_ERROR)
        {
            printf("error receiving buffer\n");
            return SOCKET_ERROR;
        }
        totalReceived += received;
        //printf("received: %d\n", received);
        //printf("totalReceived: %d\n", totalReceived);
    }
    //printf("%s", buffer);
}

I'm busy with this for 2 days now, if someone can help I will appreciate it. Thank you. How can I use both functions properly to send and receive strings without getting any errors?

This is the code. It does work when I keep it like this. When I change /LINE 1/ and /LINE 2/ to my own functions, it throws the 10014 error and closes the program.

Server:

char x[20];
printf("Give command: "); // give command to client
scanf_s("%s", x, 20);

/*LINE 1*/
int r = send(s, x, 20, 0); // let's say I send "test"
if (r == SOCKET_ERROR)
{
    printf("1 error: %d\n", WSAGetLastError());
    goto jump;
}
if(strncmp(buff, "test", 4) == 0)
{
   sendStrBuffer(s, "Hello friend!", 20); // this works
}

Client:

char buff[20];
/*LINE 2*/
int ress = recv(ClientSocket, buff, 20, 0);
if (ress == SOCKET_ERROR)
{
    printf("Server disconnected1 %d\n", WSAGetLastError());
    break;
}
if(strncmp(buff, "test", 4) == 0)
{
   char buffer[20];
   recvStrBuffer(ClientSocket, buffer, 20); // this works
   printf("%s\n", buffer); 
}

Edit (changed my code to this):

const char* recvStrBuffer(SOCKET s)
{
    char buffer[18384];
    bzero(buffer, 18384);
    int totalReceived = 0;
    int received = 0;

    // recv buffer size
    char b[sizeof(uint32_t)];
    int r = recv(s, b, sizeof(uint32_t), 0);
    if (r == SOCKET_ERROR)
    {
        printf("error recv\n");
        return "error";
    }
    uint32_t sizeBuffer = ntohl_ch(&b[0]);

    // recv buffer
    while (totalReceived < sizeBuffer)
    {
        received = recv(s, buffer + totalReceived, sizeBuffer - totalReceived, 0);
        if (received == SOCKET_ERROR)
        {
            printf("error receiving buffer %d\n",WSAGetLastError());
            return "return";
        }
        totalReceived += received;
        //printf("received: %d\n", received);
        //printf("totalReceived: %d\n", totalReceived);
    }
    //printf("%s", buffer);
    return buffer;
}
Y K
  • 71
  • 1
  • 6
  • Please provide a complete [mre]. – kaylum Jun 19 '22 at 20:36
  • 2
    Looks like your functions don't return a well-defined value except on error. That's invoking undefined behavior; in particular the caller could get back a garbage value and then use that as an offset into an array, wreaking havoc. – Jeremy Friesner Jun 19 '22 at 20:43
  • Based on your code, it seems you're trying to do a simple packet protocol that first sends the payload length, followed by the payload data. You're pretty close, so with just the code posted, I'm not sure why it's not working. Here's some similar working code from a recent answer of mine: [thread function doesnt terminate until enter is pressed](https://stackoverflow.com/a/72341219) – Craig Estey Jun 19 '22 at 20:51
  • Am I passing the buffer the right way? Maybe I should just return the buffer. Or I'm not respecting the heap and stack so maybe the adresses get changed while passing to the function? – Y K Jun 19 '22 at 21:16
  • Please provide a complete [mre] including how you are calling these functions. – kaylum Jun 19 '22 at 21:20
  • @kaylum I hope this is good – Y K Jun 19 '22 at 21:31
  • 1
    As the second comment has already told you, your functions are not returning a valid value in all code paths. So the calling code's check `if (r == SOCKET_ERROR)` could be using an indeterminate value. Do you understand that comment? At least fix that first. And that isn't really complete code. Complete code is such that anyone can take it exactly as shown to reproduce the problem. – kaylum Jun 19 '22 at 21:35
  • @kaylum I changed the function by returning a valid value, but still get the same result – Y K Jun 19 '22 at 22:11
  • `recvStrBuffer()` will go badly awry any time `int r = recv(s, b, sizeof(uint32_t), 0);` returns a value that is greater than zero but less than `sizeof(uint32_t)` -- in particular you'll end up with a nonsense/half-valid value for `sizeBuffer`, plus the remaining byte(s) of your message-size-header will be received inside the following while-loop and misused as payload-bytes instead. – Jeremy Friesner Jun 20 '22 at 05:04

0 Answers0