cimagebuffer-overflowppm

problems with buffer overflow with images in c


there! I saw a Youtube video, that was using C++ to create images. I decided to port it to C & it works like a charm. The tutorial teaches how to work with ppm files.

So then, I decided to turn it to a library, but when I run my program it gives me buffer overflow errors. That's since there is not enough memory allocated.

So my question here, is what should I replace ((img.width * img.height) * sizeof(char)) with, that can give me the exact amount of memory needed?

main.c

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

#include "image.h"

int main()
{
    image img;
    img.format = "P3";
    img.width = 250;
    img.height = 250;
    img.colour = 255;
    img.data = malloc(((img.width * img.height) * sizeof(char)));

    char numberString[4];
    for (int i = 0; i < img.height; i++)
        for (int j = 0; j < img.width; j++)
        {
            sprintf(numberString, "%d", j);
            strcat(img.data, numberString);
            strcat(img.data, " ");
            strcat(img.data, numberString);
            strcat(img.data, " ");
            strcat(img.data, numberString);
            strcat(img.data, "\n");
        }

    saveImage(img, "image.ppm");
    free(img.data);
}

image.c

#include <stdio.h>

typedef struct {char* format; int width; int height; int colour; char* data;} image;

int saveImage(image imaged, const char* fileName)
{
    // open the file
    FILE* file;
    if (!(file = fopen(fileName, "wb"))) return -1;

    // write the image
    fprintf(file, "%s\n", imaged.format);
    fprintf(file, "%d %d\n", imaged.width, imaged.height);
    fprintf(file, "%d\n", imaged.colour);
    fprintf(file, "%s", imaged.data);
    
    // close the file
    fclose(file);

    return 0;
}

Solution

  • You do not allocate enough memory.

    For every pixel you need 4 characters + one (for '\n') from time to time.

        img.data = malloc(img.width * img.height * 5 * 3); //sizeof "255" + `\n` from time to time. 3xbecause of RGB
    
    

    Additionally you do not have to use strcat which requires C string. Another error is that your function overwrites the same small amount of memory all the time.

    typedef struct
    {
        unsigned char R,G,B
    }PIXEL_t;
    
    extern PIXEL_t image[];
    
        size_t offset = 0;    
        for (int i = 0; i < img.height; i++)
            for (int j = 0; j < img.width; j++)
            {
                offset += sprintf(img.data + offset, "%hhd %hhd %hhd\n", image[i * img.width + j].R, image[i * img.width + j].G, image[i * img.width + j].B); //every pixel in new line
            }