PNG steganography tool in C

Question

This is a steganography tool enabling you to conceal any file within a PNG file. In order to compile the program you need libpng to be installed on the system. It is one of my personal projects and I would love to receive expert advice.

Running

Hide message in PNG file:

steg -h <file_in> <png_in> <png_out>

Read message from PNG file:

steg -r <png_in> <file_out>

The arguments <file_in> and <file_out> can be filenames of any (binary) files.

Code

I'm following Linus Torvalds's coding style.

In steg.c I have:

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

#include "png_io.h"

/* hide one byte in png */
void hide_byte(unsigned char byte, long i)
{
    short bit;
    for (bit = 0; bit < CHAR_BIT; ++bit) {
        long abs_bit = i * CHAR_BIT + bit;
        int y = abs_bit / channels / width;
        int x = abs_bit % (width * channels);
        png_byte *value = &row_pointers[y][x];
        if (byte & (1 << (CHAR_BIT - bit - 1)))
            *value = (*value & ~1) + 1; /* 1 */
        else
            *value = *value & ~1; /* 0 */
    }
}

/* hide file contents in png */
void hide_file(char *filename, char *src_png_name, char *out_png_name)
{
    FILE *fp = fopen(filename, "rb");
    if (!fp) {
        fprintf(stderr, "Error: failed to open file \"%s\"\n", filename);
        exit(EXIT_FAILURE);
    }

    fseek(fp, 0, SEEK_END);
    long fsize = ftell(fp);
    fseek(fp, 0, SEEK_SET);

    unsigned char buffer[4 + fsize];
    buffer[0] = fsize >> 24;
    buffer[1] = fsize >> 16;
    buffer[2] = fsize >> 8;
    buffer[3] = fsize;

    fread(&buffer[4], fsize, 1, fp);
    printf("sizeof(buffer) = %zu\n", sizeof(buffer));

    read_png(src_png_name);
    printf("width: %d\nheight: %d\nchannels: %d\n", width, height, channels);

    if ((CHAR_BIT * sizeof(buffer)) > (width * height * channels)) {
        fprintf(stderr, "Error: binary file doesn't fit into png file\n");
        exit(EXIT_FAILURE);
    }

    long i;
    for (i = 0; i < sizeof(buffer); ++i) {
        hide_byte(buffer[i], i);
    }

    write_png(out_png_name);
    fclose(fp);
}

/* read one byte from png */
void read_byte(unsigned char *byte, long i)
{
    short bit;
    for (bit = 0; bit < CHAR_BIT; ++bit) {
        long abs_bit = i * CHAR_BIT + bit;
        int y = abs_bit / channels / width;
        int x = abs_bit % (width * channels);
        png_byte *value = &row_pointers[y][x];
        if (*value & 1)
            *byte += 1 << (7 - bit);
    }
}

/* read file contents from png */
void read_file(char *filename, char *png_name)
{
    FILE *fp = fopen(filename, "wb");
    if (!fp) {
        fprintf(stderr, "Error: failed to open file \"%s\"", filename);
        exit(EXIT_FAILURE);
    }

    read_png(png_name);
    printf("width: %d\nheight: %d\nchannels: %d\n", width, height, channels);

    long fsize = 0;
    short ib;
    for (ib = 0; ib < 32; ++ib) {
        int y = ib / (width * channels);
        int x = ib % (width * channels);
        png_byte *value = &row_pointers[y][x];
        if (*value % 2 == 1)
            fsize += 1 << (31 - ib);
    }

    unsigned char buffer[4 + fsize];
    memset(buffer, 0, sizeof(buffer));
    printf("sizeof(buffer) = %zu\n", sizeof(buffer));

    long i;
    for (i = 0; i < sizeof(buffer); ++i) {
        read_byte(&buffer[i], i);
    }

    fwrite(&buffer[4], fsize, 1, fp);
    fclose(fp);
}

/* handle command line arguments */
int main(int argc, char *argv[])
{
    if (argc == 5 && !strcmp(argv[1], "-h")) {
        hide_file(argv[2], argv[3], argv[4]);
    } else if (argc == 4 && !strcmp(argv[1], "-r")) {
        read_file(argv[3], argv[2]);
    } else {
        printf("Hide message: %s -h <file_in> <png_in> <png_out>\n"
            "Read message: %s -r <png_in> <file_out>\n",
            argv[0], argv[0]);
    }
    return EXIT_SUCCESS;
}

In png_io.c I have:

#include <png.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>

#include "png_io.h"

#define PNG_BYTES_TO_CHECK 4

/* global variables */
png_infop info_ptr;
png_bytepp row_pointers;
png_uint_32 width, height;
png_byte channels;

/* read png file */
void read_png(char *filename)
{
    FILE *fp = fopen(filename, "rb");
    if (!fp) {
        fprintf(stderr, "Error: failed to open file '%s'\n", filename);
        exit(EXIT_FAILURE);
    }

    /* read signature bytes */
    unsigned char sig[PNG_BYTES_TO_CHECK];
    if (fread(sig, 1, PNG_BYTES_TO_CHECK, fp) != PNG_BYTES_TO_CHECK) {
        fprintf(stderr, "Error: failed to read signature bytes"
                "from '%s'\n", filename);
        exit(EXIT_FAILURE);
    }

    /* compare first bytes of signature */
    if (png_sig_cmp(sig, 0, PNG_BYTES_TO_CHECK)) {
        fprintf(stderr, "Error: '%s' is not a PNG file\n", filename);
        exit(EXIT_FAILURE);
    }

    /* initialize png_struct `png_ptr` */
    png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING,
            NULL, NULL, NULL);
    if (!png_ptr) {
        fclose(fp);
        fprintf(stderr, "Error: memory allocation failed\n");
        exit(EXIT_FAILURE);
    }

    /* allocate memory for image information */
    info_ptr = png_create_info_struct(png_ptr);
    if (!info_ptr) {
        fclose(fp);
        png_destroy_read_struct(&png_ptr, NULL, NULL);
        exit(EXIT_FAILURE);
    }

    /* set error handling */
    if (setjmp(png_jmpbuf(png_ptr))) {
        fclose(fp);
        png_destroy_read_struct(&png_ptr, NULL, NULL);
        exit(EXIT_FAILURE);
    }

    /* set up input control */
    png_init_io(png_ptr, fp);

    /* because we read some of the signature */
    png_set_sig_bytes(png_ptr, PNG_BYTES_TO_CHECK);

    /* read entire image into info structure */
    png_read_png(png_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, NULL);

    /* optain information */
    row_pointers = png_get_rows(png_ptr, info_ptr);
    width = png_get_image_width(png_ptr, info_ptr);
    height = png_get_image_height(png_ptr, info_ptr);
    channels = png_get_channels(png_ptr, info_ptr);

    /* free allocated memory */
    png_destroy_read_struct(&png_ptr, NULL, NULL);
    fclose(fp);
}

/* write png file */
void write_png(char *filename)
{
    FILE *fp = fopen(filename, "wb");
    if (!fp) {
        fprintf(stderr, "Error: failed to open file '%s'\n", filename);
        exit(EXIT_FAILURE);
    }

    /* initialize png_struct `png_ptr` */
    png_structp png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING,
            NULL, NULL, NULL);
    if (!png_ptr) {
        fclose(fp);
        fprintf(stderr, "Error: memory allocation failed\n");
        exit(EXIT_FAILURE);
    }

    /* set up output control */
    png_init_io(png_ptr, fp);

    /* save new pixel values */
    png_set_rows(png_ptr, info_ptr, row_pointers);

    /* all image data in info structure */
    png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, NULL);

    /* free allocated memory */
    png_destroy_write_struct(&png_ptr, &info_ptr);
    fclose(fp);
}

In png_io.h I have:

#ifndef PNG_IO_H
#define PNG_IO_H

#include <png.h>

/* global variables */
extern png_bytepp row_pointers;
extern png_byte channels;
extern png_uint_32 width, height;

/* read png file */
void read_png(char *filename);

/* write png file */
void write_png(char *filename);

#endif /* PNG_IO_H */

Answer 1

Unconditional masks

This:

    if (byte & (1 << (CHAR_BIT - bit - 1)))
        *value = (*value & ~1) + 1; /* 1 */
    else
        *value = *value & ~1; /* 0 */

is really just

*value &= ~1;
if (byte & (1 << (CHAR_BIT - bit - 1)))
    *value |= 1;

Const arguments

void hide_file(char *filename, char *src_png_name, char *out_png_name)

should be

void hide_file(const char *filename, const char *src_png_name, const char *out_png_name)

C99

It's nice. It will allow this:

long i;
for (i = 0; i < sizeof(buffer); ++i) {

to be

for (long i = 0; i < sizeof(buffer); ++i) {

Or vs. add

I suspect that these:

        *byte += 1 << (7 - bit);
        *value = (*value & ~1) + 1; /* 1 */

are more safely expressed as bitwise or | operations, and really that better communicates your intent anyway.