malloc(): corrupted top size

59,609

You have a buffer overflow in newfilename:

/* here you declare that the string pointed to by newfilename may contain `i`
   chars + zero-termination...*/ 
newfilename=calloc(i+1,1);

/* Here `i` is less than `strlen(filename)`, so only i characters from `filename`
   will be copied, that does NOT include the zero-termination
   (but it will be zero-terminated because the last byte in the buffer was 
   already cleared by `calloc`) */ 
strncpy(newfilename,filename,i);


/* Here `newfilename` is full, and `strcat` will write outside buffer!! Whatever
   happens after this line is undefined behaviour */   
strcat(newfilename,"(decrypted).txt");

So you have to allocate enough space for newfilename:

newfilename = calloc (i + strlen("(decrypted).txt") + 1, 1);
Share:
59,609
Hakan Demir
Author by

Hakan Demir

Updated on June 16, 2020

Comments

  • Hakan Demir
    Hakan Demir about 4 years

    I am working on a program that decrypts some line of text in a file. First, another source code I created asks for shift and some text. Source code encrypts the text and writes it into a file.

    Then, I try to decrypt the specified file using the below code -another source code. Name of the file is obtained. From file name, shift is obtained. Every character written inside the file is copied, shifted by the value of shift with the aid of the function caesar_decrypt -reversing caesar encrypt which I also have. However, given characters must be of ascii value between 32 and 127 to be transformed to another character of ascii value between 32 and 127.

    I defined my modulo function encapsulating the remainder operator -modulo of positive numbers- and modulo of negative numbers. I also defined strip function which works the same as strip function in python does. Aside from spaces, it removes all literals. I also have getIn(), which is input() of python.

    I tried to debug by printing and using gdb. No effective result. I have researched about this topic in stackoverflow. Found one entry. Answer did not address my specific issue.

    NOTE: I copied the whole program since I thought you would need the necessary detailed information.

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <math.h>
    #include <unistd.h>
    #include <fcntl.h>
    
    /*GETIN FUNCTION*/
    char* getIn() 
    {
        char *line = NULL, *tmp = NULL;
        size_t size = 0, index = 0;
        int ch = EOF;
    
        while (ch) 
        {
            ch = getc(stdin);
    
            /* Check if we need to stop. */
            if (ch == EOF || ch == '\n')
            ch = 0;
    
            /* Check if we need to expand. */
            if (size <= index) 
            {
                size += sizeof(char);
                tmp = realloc(line, size);
                if (!tmp) 
                {
                    free(line);
                    line = NULL;
                    break;
                }
            line = tmp;
            }
    
        /* Actually store the thing. */
        line[index++] = ch;
        }
    
        return line;
    }
    /*FUNCTION THAT CONVERTS TO NUMERIC CHARACTER, PRECURSOR OF DIGITS OF SHIFT*/
    char make_numeric(char ch)
    {
        if(ch<91 && ch>63) return ch-16;
        if(ch<123 && ch>95) return ch-48;
        if(ch<58 && ch>47) return ch;
        return ch+16;
    }
    
     /*STRIP FUNCTION*/
    char* strip(char* str,int length)
    {
        char *start=str,*end=&str[length-1];
    
        while(*start>0 && *start<33 || *start==127) ++start;
        if(!*start) return start;
    
        while(*end>=0 && *end<33 || *end==127) --end;
        ++end;
        *end=0;
    
        return start;
    }
    /*DECRYPTOR FUNCTION*/
    char *caesar_decrypt(char *message_to_decrypt, void *shift)
    {   
        int length=strlen(message_to_decrypt),i;
        char* decrypted_message;
    
        if(!message_to_decrypt) return NULL;
    
        if(!(decrypted_message=calloc(length+1,sizeof(char)))) return NULL;
    
        for(i=0;i<length;++i)
        {
        decrypted_message[i]=mod((message_to_decrypt[i]-33-*((int*)shift)),94)+33;
        }
    
    return decrypted_message;
    }   
    
    /*THE MAIN PROGRAM*/
    int main()
    {
    
    int shift,len_chshift,init,len_filename,i=0;
    char *chshift,*line=NULL,*decrypted_line,*stripped_line,chinit,chlen_chshift;
    char *filename=NULL,*newfilename=NULL;
    FILE *oldfile=NULL;
    FILE *newfile=NULL;
    size_t lenline,len=0;
    
    printf("-9");
    /*FILENAME*/
    printf("Enter the file name: ");
    /*printf("-8");*/
    filename=getIn();
    /*printf("-7");*/
    if(!access(filename,F_OK))
    {
        len_filename=strlen(filename);
        /*printf("-6");*/
        i=len_filename;
        while(filename[i]!='.') --i;        
        chlen_chshift=filename[i-1];
        chlen_chshift=make_numeric(chlen_chshift);
        len_chshift=chlen_chshift-48;
        /*printf("-5");*/
        chshift=calloc(len_chshift+1,1);
        /*NEWFILENAME*/
        newfilename=calloc(i+1,1);
        /*printf("-4");*/
        strncpy(newfilename,filename,i);
        /*printf("-3");*/
        strcat(newfilename,"(decrypted).txt");
        /*printf("-2");*/
        chinit=make_numeric(filename[0]);
        init=chinit-48;
        /*SHIFT*/
        i=0;
        /*printf("-1");*/
        while(i!=len_chshift)
        {
            chshift[i]=make_numeric(filename[(i+1)*init]);
            ++i;
        }
        /*printf("0");*/
        shift=atoi(chshift);
    
        /*printf("1");*/
        if(!(oldfile=fopen(filename,"r")))
        {
            perror("Error");
            if(newfilename) free(newfilename);
            if(filename) free(filename);
            if(chshift) free(chshift);
            exit(1);
        }
        /*printf("2");*/
    
        if(!(newfile=fopen(newfilename,"+ab")))
        {
            perror("Error");
            if(newfilename) free(newfilename);
            if(filename) free(filename);
            if(chshift) free(chshift);
            fclose(oldfile);
            exit(1);
        }
        while ((lenline = getline(&line, &len, oldfile)) != -1)
        {
            stripped_line=strip(line,lenline);
            decrypted_line=caesar_decrypt(stripped_line, &shift);
            if(!decrypted_line)
            {
                printf("Could not allocate memory\n");
                if(newfilename) free(newfilename);
                if(filename) free(filename);
                if(chshift) free(chshift);
                exit(1);
            }
    
            fprintf(newfile,"%s\n",decrypted_line);
    
            if(decrypted_line) free(decrypted_line);
            decrypted_line=NULL;
    
            free(line);
            line=NULL;
            stripped_line=NULL;
            decrypted_line=NULL;
        }
        free(line);
        line=NULL;
    }
    else
    {
        printf("Cannot access the file.");
        if(filename) free(filename);
        exit(0);
    }
    
    if(newfilename) free(newfilename);
    if(filename) free(filename);
    if(chshift) free(chshift);
    
    fclose(oldfile);
    fclose(newfile);
    return 0;
    }
    

    The program is required to create a file, context of which is the decrypted clone of the other file, name of which is provided in the beginning by the user. However, it throws, just before the first fopen:

    malloc(): corrupted top size
    Aborted (core dumped)
    

    When I uncomment every printf line, it gives the same error between the lines of getIn() and printf("-7");. When printfs are commented, I observe that filename holds the written value, which is returned by getIn(). Therefore, I do not think getIn() is the problem.

    EDIT(1): I added 2 headers that were missing, but still raising the same error.

    EDIT(2): I removed all unnecessary headers.

    EDIT(3): Since I am requested, I will try to illuminate by an example: Suppose I have a file named

    2/b*A".txt
    

    with two lines of code

    %52
    abcd
    

    I enter the file name into program.

    1)The very last character before . is ", converted into number 2. We understand shift is a two digit number.

    2)The first character is the initiator, which is 2 , is the number of characters we need to jump repeatedly. We will jump two characters twice since we have a shift number of two digits.

    3)We have, now characters b and A, converted to 2 and 1 respectively. Our shift is 21. The very first character %(37) is shifted backwards to n(110) and so on, we need to create a new file with the content:

    n~#
    LMNO
    

    I appreciate any help. Thank you in advance. In addition, the variables I mentioned are any arbitrary characters between specific intervals. For details, check the source code,or I suggest you to work locally, since I provided with the whole program.

    For ascii codes: https://www.asciitable.com/

    If you are interested in Caesar's cipher: https://en.wikipedia.org/wiki/Caesar_cipher