dlt-daemon icon indicating copy to clipboard operation
dlt-daemon copied to clipboard

String overflows

Open averater opened this issue 1 year ago • 3 comments

There are multiple possible string overflow issues. One recent commit tried to solve two cases but used the wrong length: https://github.com/COVESA/dlt-daemon/commit/0d76220d78e0dd2e1139058e2e96e35e8a72c753

On my compiler I get this warning: stringop-overflow.

This should be solved by either double-checking all strcpy and strncpy so the buffer definitely have enough space for the trivial cases with known lengths or by checks of available buffer for the non-trivial cases

Here is some code to explain the issues:

#include <string.h> 
#include <stdio.h>
#include <stdlib.h>

int avail(char buf[], int l) {
    int ret = l - strlen(buf) - 1;
    if (ret<0) return 0;
    return ret;
}
int main(void) 
{
    {
        char str1[4] = "abc";
        char str2[5] = "def";
        strcat(str1, str2);
        strcat(str1, "...");
        puts(str1);  // Buffer overflow!
    }{
        char str1[4] = "abc";
        char str2[5] = "def";
        strncat(str1, str2, strlen(str2));
        strncat(str1, "...", 3);
        puts(str1);  // Buffer overflow!
    }{
        char str1[8] = "abc";
        int l = sizeof(str1)/sizeof(*str1);
        char str2[3] = "def";  // not null terminated
        strncat(str1, str2, avail(str1, l));
        strncat(str1, "...", avail(str1, l));
        puts(str1);  // Undefined
    }{
        char str1[8] = "abc";
        int l = sizeof(str1)/sizeof(*str1);
        char str2[4] = "def";
        strncat(str1, str2, avail(str1, l));
        strncat(str1, "...", avail(str1, l));
        puts(str1);  // OK
    }
}

averater avatar Oct 14 '24 13:10 averater

Hello @averater , thanks for the fix but please do not upload binary (zip files) files here. I just deleted it. Can you please open a pull request?

michael-methner avatar Oct 18 '24 10:10 michael-methner

dlt-daemon.diff.txt There are a lot of places where this should be fixed to stop the warnings. Most are of the trivial kind where I just replaced strncat with strcat. But since it is fairly large I'd prefer to not drive a pull request. I uploaded as a txt file instead.

averater avatar Oct 24 '24 08:10 averater

Got time after my other issues were done so here are two pull requests: https://github.com/COVESA/dlt-daemon/pull/697 https://github.com/COVESA/dlt-daemon/pull/696

averater avatar Oct 29 '24 07:10 averater