C - Error on free()?

Question

I am making some hash function.

It's source code is ...

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

int m_hash(char *input, size_t in_length, char *output, size_t out_length);

int main()
{
    char buffer[1025];
    char output[65];
    output[64] = '\0';
    while(1)
    {
        printf("enter what will hash : ");
        fgets(buffer, 1024, stdin);
        buffer[1024] = '\0';
        m_hash(buffer, strlen(buffer), output, 64);
        printf("your string (hashed) : %s\n", output);
    }
    return 0;
}

int m_hash(char *input, size_t in_length, char *output, size_t out_length) // 0 on Success, 1 on Fail.
{
    const char OUT[64] = "zxcvbnmasdfghjklqwertyuiopZXCVBNMASDFGHJKLQWERTYUIOP1234567890_-";
    char *snow = (char *)malloc(sizeof(char) * out_length);
    if (snow == NULL) return 1;
    int *out = (int *)malloc(sizeof(int) * out_length);
    if (out == NULL)
    {
        free(snow);
        return 1;
    }
    memset(out, 0x00, sizeof(int) * out_length);
    size_t tmp = 0, i, j;
    for (i = 0; i < out_length; i++)
    {
        tmp += i * i;
        snow[i] = tmp / (i + 1);
        tmp -= snow[i];
    }
    for (i = 0; i < in_length; i++)
    {
        out[((tmp *= (i + 1)) % out_length + out_length) % out_length] += input[i];
        snow[i] += input[i];
        for (j = 0; j < out_length; j++)
        {
            out[((i + j) % out_length + out_length) % out_length] += snow[j] % (i + 1);
        }
    }
printf("Will free snow.\n");
    free(snow); // ERROR WTF?
printf("Freed snow.\n");
    for (i = 0; i < out_length; i++)
    {
        output[i] = OUT[(out[i] % 64 + 64) % 64];
    }
    free(out);
    return 0;
}

When I enter short string, there is no error. But when I enter long string like this... It's seems to free() makes error... There is error on 'free(snow);'?!?!!

Visual studio's debug message is... HEAP CORRUPTION DETECTED: after Normal block (#92) at 0x01505D08. CRT detected that the application wrote to memory after end of heap buffer.

I really don't know what's wrong with this...

What's wrong...? ㅠ.ㅜ

Question: How do you plan to break out of the `while(1)` loop? — Sourav Ghosh, May 17 '17 at 07:42
[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, May 17 '17 at 07:44
@SouravGhosh The program is not finished yet... I will add break :D :D — You, May 17 '17 at 07:45
If `in_length > out_length` the code `snow[i] += input[i];` access `snow` out of bounds. — LPs, May 17 '17 at 07:45
`out[((tmp *= (i + 1)) % out_length + out_length) % out_length] += input[i];`..seriously?? and you expect ___people___ to read your code? — Sourav Ghosh, May 17 '17 at 07:46
@LPs omg... I can't understand why I haven't seen it... Thanks! — You, May 17 '17 at 07:50

score 0 · Accepted Answer · answered May 17 '17 at 07:46

0

Without debugging the exact place where it happens -> at some point you're writing before/after the memory you allocated. This overwrites the allocation metadata which doesn't affect your program while it's running, but when you try to free the memory, you'll find garbage and crash.

If you were running linux I'd recommend trying valgrind to debug this, but apparently there are some Windows options as well: Is there a good Valgrind substitute for Windows?

Basically you want something which will error out the moment you write out of bounds, not when you try to read it back.

answered May 17 '17 at 07:46

viraptor

33,322
10
107
191

4

With all due respect, this is more suitable as a comment. – Sourav Ghosh May 17 '17 at 07:47
@SouravGhosh It's too long for a comment and it answers the question. As you mentioned yourself - was anybody expected to actually read that code? – viraptor May 17 '17 at 08:03

score 0 · Answer 2 · answered May 17 '17 at 08:06

The issue comes from the insufficient malloc'd snow buffer. You malloc it the size out_length (char *snow = (char *)malloc(sizeof(char) * out_length);).

Though you loop over it with in_length (wich is up to 1024 while out_length is 64):

for (i = 0; i < in_length; i++)
{
    out[((tmp *= (i + 1)) % out_length + out_length) % out_length] += input[i];
    snow[i] += input[i];
    for (j = 0; j < out_length; j++)
    {
        out[((i + j) % out_length + out_length) % out_length] += snow[j] % (i + 1);
    }
}

At the third line of this block, when i > 64, you get a memory error most likely causing a memory corruption making your program crash at free().

score 0 · Answer 3 · answered May 17 '17 at 08:20

As pointed by the comments above, there would be a greater possibility that in_length > out_length. Which also supports your facts that it will crash for very long strings.

So with that, you may want to support increasing the size of snow via realloc prior to for (i = 0; i < in_length; i++)

if (in_length > out_length) {
    int additional_bytes =  in_length - out_length;
    snow = realloc(snow, sizeof(char) * (out_length + additional_bytes));
}

C - Error on free()?

3 Answers3