buffer overflow during memory reallocation

Question

I was making a C program to convert 12 hour clock into 24 hour clock with the format of input being HH:MM:SSAM or HH:MM:SSPM and 24hr clock output being HH:MM:SS

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

int main(){
    char *time = malloc(11 * sizeof(char));
    scanf("%11s", time);
    if (time[8] == 'A' || time[8] == 'P')
    {

        if (time[8] == 'A')
        {
            time = realloc(time, (9 * sizeof(char)));                
            printf("%s\n", time);
        }
        else
        {
            time = realloc(time, (8 * sizeof(char)));                
            char str[3];
            sprintf(str, "%c%c", time[0], time[1]);
            int hours;
            hours = atoi(str);
            int milhours;
            milhours = hours + 12;
            char milstr[3];
            sprintf(milstr, "%d", milhours);
            time[0] = milstr[0];
            time[1] = milstr[1];
            printf("%s\n", time);
        }   
    }
    else
    {
        printf("give a standard format\n");
        return 0;
    }
    return 0;
}

There is no compilation error,but the program doesnt run because of buffer overflow. When I reduce the size of dnamic array time , is it necessary that the last 2 elements of time will be removed?

EDIT: I updated time, str and milstr for NULL terminator and the buffer overflow problem is resolved. Thanks for the recommended reading!

Editing your question in place is not a good idea after comments and/or questions have been given as such edits may render the answers/comment und-understandable. Better add edits as updates/additions to your question. — alk
– alk, Commented Jan 13, 2017 at 21:01

Schwern · Accepted Answer · 2017-01-13 21:49:47Z

3

EDIT: I updated time, str and milstr for NULL terminator and the buffer overflow problem is resolved. Thanks for the recommended reading!

You still have problems.

char *time = malloc(11 * sizeof(char));
scanf("%11s", time);

That malloc allocates 11 bytes, but that scanf needs 12. That's because C strings are terminated with a null byte, you always need to allocate one more byte.

As for all the reallocating you're doing, it seems like you're doing that to truncate the string. That's overkill for 3 bytes, and it won't truncate anyway. Because realloc may return the same pointer, you can't expect that shrinking will zero the following memory and truncate the string. You can't expect it when it returns a different pointer either, or when it grows memory. Only with calloc can you be certain that the allocated memory has been zeroed.

Instead, skip the realloc and stick a null byte in to truncate the string.

if (time[8] == 'A')
{
    time[8] = '\0';
    puts(time);
}

time is still 12 bytes (once the malloc has been fixed), but the null byte tells C to stop reading at time[8].

edited Jan 13, 2017 at 21:49

answered Jan 13, 2017 at 21:44

Schwern

167k28 gold badges225 silver badges370 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

kartikeykant18 Over a year ago

thanks for the answer! I found out these problems maybe at the same time you were, and I fixed those, my main issue was buffer overflow though which I got an idea through comments :)

Collectives™ on Stack Overflow

buffer overflow during memory reallocation

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related