Search code examples
ciobuffer-overflowflawfinder

read() - Check buffer boundaries if used in a loop including recursive loops


I have this code and run it with Flawinder, and i get this output on the read() functions:

Check buffer boundaries if used in a loop including recursive loops

Can anyone see the problem?

#include <stdlib.h>
void func(int fd)
{

char *buf;
size_t len;
read(fd, &len, sizeof(len));

if (len > 1024)
return;
buf = malloc(len+1); 
read(fd, buf, len); 
buf[len] = '\0';
}

Solution

  • you should check the return value of read() to know whether call to read() was success or failure or if read() was interrupted by a signal then set the errno. For e.g

    ssize_t ret = read(fd, &len, sizeof len);
    if( (ret == -1 || ret != sizeof len) {
       /* error handling @TODO */
    }
    

    Most importantly here

    ret = read(fd, buf, len); /* read() may read less than len characters */ 
    

    read() returns the number of bytes read, so instead of this

    buf[len] = '\0';
    

    use

    buf[ret] = '\0'; /* correct way */
    

    Sample Code

    void func(int fd) { /* assume fd is a valid file descriptor */
            char *buf = NULL;
            size_t len;
            errno = 0; /* set this to 0 */
            ssize_t ret = read(fd, &len, sizeof len);
            if( (ret == -1 || ret != sizeof len) {
                    /* error handling @TODO */
            }
            if (len > 1024) {
                    return;
            }
            buf = malloc(len+1); 
            if(buf == NULL) {
                    /* error handling @TODO */
            }
            ret = read(fd, buf, len);
            if(ret!=-1) {
                    buf[ret] = '\0';
                    /* do something with buf and free it once usage is done*/
            }       free(buf); /* free the buf */
            else { /* if read failed */
                    free(buf); /* free the buf */
            }
    }