Simple program to hide messages in files (steganography)

Question

That is a simple program to hide text messages in text files. I'd like to hear any advices to improve my code and style.

Some notes:

1. There is no error handling to simplify the program. It is not a «real» program anyway. There is many assertions instead error handling.
2. I wrote my own implementations to strdup and strrev because these functions are not a part of ISO C. I called they as reverseString and duplicateString corresponding since names like str* is reserved by ISO C.
3. I check the validity of input arguments in functions to prevent mistakes of using internal API.
4. All the functions marked as static because all they in a single translation unit.

And one another question: the input file should be opened in text mode while temporary file is always opens in binary mode (w+b). Could it lead to some problems?

Compiler

I'm using Clang to compile the program with such flags:

-std=c11
-Weverything
-Wpedantic
-fsanitize=address
-D_CRT_SECURE_NO_WARNINGS

It gives only one warning:

warning: implicit conversion turns floating-point number into integer: 'double' to 'size_t' (aka 'unsigned int') [-Wfloat-conversion]
        const size_t newCapacity = ceil(s->capacity * DYNAMIC_STRING_GROW_FACTOR);

Static code analyzer

Also I have checked the code by CppCheck and it gives only one error: «Memory leak: ret in function stringToBinary»:

char *stringToBinary(const char *s)
{
    assert(s);
    assert(strlen(s) > 0);

    char *ret = calloc((strlen(s) + 1) * CHAR_BIT + 1, 1);
    assert(ret);

    for (size_t i = 0; s[i] != '\0'; i++)
        strcat(ret, charToBinary(s[i]));

    return strcat(ret, charToBinary('\0'));
}

But I think it is a false positive because ret is freed in the hideMessage function:

char *msgBinary = stringToBinary(msg);
...
free(msgBinary);

---

I'll be glad if you give me any tips to improve my code or correct my mistakes. Thanks you in advance! Best regards :)

#include <math.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <limits.h>
#include <assert.h>

#define DYNAMIC_STRING_GROW_FACTOR 1.5

typedef struct DynamicString {
    char *s;
    size_t capacity;
    size_t length;
} DynamicString;

static void  createDynamicString(DynamicString *, size_t);
static void  destroyDynamicString(DynamicString *);
static void  increaseDynamicStringCapacity(DynamicString *);
static void  appendCharToDynamicString(DynamicString *, char);
static void  hideMessage(FILE *, const char *);
static char *extractMessage(FILE *);
static void  copyFile(FILE *, FILE *);
static char *stringToBinary(const char *);
static char *charToBinary(char);
static char  charFromBinary(const char *);
static char *reverseString(char *);
static char *duplicateString(const char *s);

int main(void)
{
    FILE *file = fopen("file.txt", "r+");
    assert(file);

    hideMessage(file, "hello, world");

    char *msg = extractMessage(file);
    assert(msg);
    puts(msg);

    free(msg);
    fclose(file);
}

/* The HIDEMESSAGE function
 *
 * The function hides text message into the file by that way: 1) the message
 * converts to binary; 2) each bit of the converted message writes at the end
 * of each file's line (right before the new-line character): if the bit
 * is 1 then a space (' ') appends to the line otherwise it's nothing appends
 * to the line.
 *
 * Assumes the the file does not contain any spaces right before new-line
 * characters. Also assumes that the file has enough lines for storing
 * the message.
 */
void hideMessage(FILE *f, const char *msg)
{
    assert(f);
    assert(msg);
    assert(strlen(msg) > 0);

    char *msgBinary = stringToBinary(msg);
    assert(msgBinary);

    FILE *tempFile = tmpfile();
    assert(tempFile);

    for (int ch, i = 0; (ch = fgetc(f)) != EOF;) {
        if (msgBinary[i] && (ch == '\n'))
            if (msgBinary[i++] == '1')
                fputc(' ', tempFile);

        fputc(ch, tempFile);
    }

    copyFile(f, tempFile);

    free(msgBinary);
    fclose(tempFile);
}

/* The EXTRACTMESSAGE function
 *
 * The function extracts message hidden by the HIDEMESSAGE function from
 * the input file and returns a pointer to heap-allocated string which
 * contains the message.
 */
char *extractMessage(FILE *f)
{
    assert(f);

    DynamicString msgBuffer;
    createDynamicString(&msgBuffer, 128);

    char charInBinary[CHAR_BIT + 1] = {0};

    for (int prevCh = 0, ch, i = 0; (ch = fgetc(f)) != EOF; prevCh = ch) {
        if (ch == '\n')
            charInBinary[i++] = (prevCh == ' ') ? '1' : '0';

        if (i % CHAR_BIT == 0 && i != 0) {
            if (!strcmp(charInBinary, charToBinary('\0')))
                break;

            i = 0;
            appendCharToDynamicString(&msgBuffer, charFromBinary(charInBinary));
        }
    }

    char *ret = duplicateString(msgBuffer.s);
    assert(ret);

    destroyDynamicString(&msgBuffer);
    return ret;
}

/* The CREATEDYNAMICSTRING function
 *
 * The function initializes a DynamicString passing by the first argument.
 * The initial capacity of the string is passing by the second argument.
 * Capacity is the max length of the string. At the same time length is
 * current length of the string. Thus the function allocates capacity + 1
 * bytes for the string (considering the null-character).
 *
 * The input pointer to DynamicString struture should be a valid pointer and
 * capacity should be greater than 0.
 */
void createDynamicString(DynamicString *ret, size_t capacity)
{
    assert(ret);
    assert(capacity > 0);

    ret->s = malloc(capacity + 1);
    assert(ret->s);

    ret->length = 0;
    ret->capacity = capacity;
}

/* The APPENDCHARTODYNAMICSTRING function
 *
 * The function appends a character to the input DynamicString. If capacity of
 * the string is not enough the function increases it.
 *
 * The input pointer to a DynamicString should be a valid pointer as well as
 * its string buffer.
 */
void appendCharToDynamicString(DynamicString *s, char c)
{
    assert(s);
    assert(s->s);

    if (s->length == s->capacity)
        increaseDynamicStringCapacity(s);

    s->s[s->length++] = c;
    s->s[s->length] = '\0';
}

/* The INCREASEDYNAMICSTRINGCAPACITY function
 *
 * The function increases capacity of the input DynamicString. Grow factor
 * is sets by a macro constant DYNAMIC_STRING_GROW_FACTOR.
 *
 * The input pointer to a DynamicString struture should be a valid pointer
 * as well as its string buffer.
 */
void increaseDynamicStringCapacity(DynamicString *s)
{
    assert(s);
    assert(s->s);

    const size_t newCapacity =  ceil(s->capacity * DYNAMIC_STRING_GROW_FACTOR);

    s->s = realloc(s->s, newCapacity + 1);
    assert(s->s);

    s->capacity = newCapacity;
}

/* The DESTROYDYNAMICSTRING function
 *
 * The function destroys the input DynamicString. It frees the string buffer
 * of the input DynamicString.
 *
 * The input pointer to a DynamicString should be a valid pointer as well as
 * its string buffer.
 */
void destroyDynamicString(DynamicString *s)
{
    assert(s);
    assert(s->s);

    free(s->s);
}

/* The COPYFILE function
 *
 * The function copies all the contents of src to dest. Both arguments should
 * be valid pointers. dest should be open for writing, src should be open
 * for reading. The function does not close the files. The both file cursor
 * position sets to the beginning.
 */
void copyFile(FILE *dest, FILE *src)
{
    assert(dest);
    assert(src);

    rewind(dest);
    rewind(src);

    for (int ch; (ch = fgetc(src)) != EOF;)
        fputc(ch, dest);

    rewind(dest);
    rewind(src);
}

/* The CHARFROMBINARY function
 *
 * The function converts the input string returned by the CHARTOBINARY function
 * to a character.
 *
 * The input string should be a valid null-terminated string and its length
 * should be greater 0.
 *
 * charFromBinary(charToBinary(c)) == c
 */
char charFromBinary(const char *s)
{
    assert(s);
    assert(strlen(s) > 0);

    char ret = 0;
    unsigned int p = 1;

    for (size_t i = strlen(s); i-- > 0; p *= 2)
        if (s[i] == '1')
            ret += p;

    return ret;
}

/* The STRINGTOBINARY function
 *
 * The function converts the input string to binary form and returns a pointer
 * to heap-allocated null-terminated string. Null-terminator of the input
 * string also converts to binary form and appends to the result. The caller
 * should free memory allocated for the output string.
 *
 * The input string should be a valid null-terminated string and its length
 * should be greater 0.
 *
 * stringToBinary("cat") => "01100011011000010111010000000000"
 * stringToBinary("dog") => "01100100011011110110011100000000"
 * stringToBinary("R\0") => "0101001000000000"
 */
char *stringToBinary(const char *s)
{
    assert(s);
    assert(strlen(s) > 0);

    char *ret = calloc((strlen(s) + 1) * CHAR_BIT + 1, 1);
    assert(ret);

    for (size_t i = 0; s[i] != '\0'; i++)
        strcat(ret, charToBinary(s[i]));

    return strcat(ret, charToBinary('\0'));
}

/* The CHARTOBINARY function
 *
 * The function converts value of the input character to binary form and
 * returns a pointer to the static null-terminated string which contains
 * the result. The result contains leading zeroes.
 *
 * charToBinary(100) => "01100100"
 * charToBinary('A') => "01000001"
 * charToBinary('\0') => "00000000"
 */
char *charToBinary(char c)
{
    static char ret[CHAR_BIT + 1];

    memset(ret, '0', sizeof ret);
    ret[sizeof ret - 1] = '\0';

    for (size_t i = 0; c; i++) {
        ret[i] = (c % 2) ? '1' : '0';
        c /= 2;
    }

    return reverseString(ret);
}

/* The REVERSESTRING function
 *
 * The input string should be a valid pointer to a null-terminated string.
 * If the input string is empty the function does noting.
 *
 * reverseString("hello") => "olleh"
 * reverseString("a") => "a"
 * reverseString("") => "a"
 */
char *reverseString(char *s)
{
    assert(s);

    char *begin = s;
    char *end = s + strlen(s) - 1;

    for (; begin < end; begin++, end--) {
        const char t = *begin;
        *begin = *end;
        *end = t;
    }

    return s;
}

/* The DUPLICATESTRING function
 *
 * The function returns a pointer to a heap-allocated string, which is a
 * duplicate of the input string. The returned pointer can be passed to the
 * free function.
 */
char *duplicateString(const char *s)
{
    assert(s);
    assert(strlen(s) > 0);

    char *copy = malloc(strlen(s) + 1);
    assert(copy);

    return strcpy(copy, s);
}

_{P.S. I'm not a native English speaker so… sorry for the inconvenience.}

Answer 1

Here are some things that may help you improve your program.

Don't hardcode file names

Generally, it's not a good idea to hardcode a file name in software. Instead, it would be better to allow the user of the program to specify the name, as with a command line parameter.

Don't use assert to handle errors

The use of assert is to help debug the program, so in some cases, such as assert(f) in hideMessage it makes sense, but in others, such as assert(file) in main(), it's not appropriate. Better would be to write something like this:

if (infile == NULL) {
    fprintf(stderr, "Error: could not open input file \"%s\"\n", argv[1]);
    return 2;
}

Don't bother encoding terminating NUL character

By definition, the string-terminating NUL character ('\0') is a string of zeroes which will, according to the encoding rules, not alter the line endings. For that reason, there's little reason to bother encoding them.

Allow separation of input and output files

It may be that having the input and output files as separate files would be handy. For that reason, I'd suggest making them separate file names.

Rethink the use of strings

The conversion of each message string to a string that's eight times longer (by converting into the equivalent string of '1' and '0' characters is not really necessary. One could instead simply pick off bits of the message when encoding. Here's one way to do that:

bool hideMessage(const char *msg, FILE *infile, FILE *outfile) {
    if (msg == NULL) {
        return false;
    }
    for ( ; *msg; ++msg) {
        for (int mask = 0x80; mask; mask >>= 1) {
            if (encodeBit(infile, outfile, mask & *msg) == EOF) {
                return false;
            }
        }
    }
    // copy the rest of the file
    for (int ch = fgetc(infile); ch != EOF; ch = fgetc(infile)) {
        fputc(ch, outfile);
    }
    return true;
}

This marches through the passed message, encoding bit at a time until it encounters the end of the input message and then simply copies the rest of the input file to the output file unaltered. Here's one way to write the encodeBit function:

int encodeBit(FILE *infile, FILE *outfile, char bit) {
    for (int ch = fgetc(infile); ch != EOF; ch = fgetc(infile)) {
        switch(ch) {
            case '\n':
                if (bit) {
                    fputc(' ', outfile);
                }
                fputc(ch, outfile);
                return ch;
                break;
            default:
                fputc(ch, outfile);
        }
    }
    return EOF;
}

This code returns the last character read, so that EOF can be used as an indication of error (that is, the file is too short to encode the message).

One can also create the reverse functions in very similar manner.

Be aware of file modes differences

On POSIX compliant operating systems, such as Linux, there is no difference between binary and text mode for file handles. However other operating systems, most notably Windows, do differentiate between them. The difference is that CR LF (0x0d 0x0a) is used as the line ending in Windows (while just LF, 0x0a is used on Linux). For that reason, for code like this, I'd suggest opening both files in text mode. This means, unfortunately, that you can't use tmpfile() and follow this advice. Otherwise you could have the unfortunate situation in which the file is emitted with CR SP LF (0x0d 0x20 0x0a) which would likely confuse Windows greatly.

Comments on comments

Although a native English speaker will likely be able to detect from the comments that they were not written by a native English speaker, they're still quite understandable and convey exactly the kinds of information that a reader needs -- "what does it do and how should I use it?" My only complaint, and it's a small one, is that starting each comment block with "The APPENDCHARTODYNAMICSTRING function" is not helpful. First, we know it's a function so that doesn't add much information, and second, it turns the relatively readable appendCharToDynamicString and turned it into something less readable. I tend to document my C code with annotations that can be used by Doxygen which can then be used to very easily turn your source code into high quality documentation.

Consider enhancements

It might be useful to encrypt the string before encoding. It might also be useful to provide the ability to detect if the file already might contain a hidden message. This could be done by always encoding a 1 bit before every message. Further, if the code used space and tab characters to encode ones and zeroes, it would be possible to stack multiple messages in the same file.