malloc(): corrupted top size

Question

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

Cuticle

Comments are not for extended discussion; this conversation has been moved to chat. – Wenzel

@HakanDemir You cannot have filenames with /s in them. – Nicotinism

Answer 1

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);

Stgermain

There is chance that newfilename is not null terminated after strncpy and doing strcat onnewfilename is undefined behavior. – Frier

@kiran. No, newfilname will be zero terminated. Because of calloc() all bytes will be zero before strncpy(), and we copy max i bytes while buffer holds i+1 bytes. – Stgermain

Oops I didn't see that part.UV for heads-up. – Frier

@kiran, The comment i made above was wrong (long time since I wrote the answer). But the claim stands, 'newfilename` will be zero terminated when using calloc (i + strlen("(decrypted).txt") + 1, 1). Then the buffer will be large enough for strncpy(), strcat() and zero termination. The buffersize of i+1 was to point out the bug in the original code. – Stgermain

Answer 2

I encountered a similar print of

malloc(): corrupted top size
Aborted

It took me some time to find the issue as, although it was raised by a call to "new myClass();" call, there wasn't a clear reason for this issue, and no use of string or array/buffer/memory copy to cause it.

TLDR: the use of #pragma pack(push,1) in the ICD file without the following #pragma pack(pop) resulted in the same behavior as described in the issue raised in this thread.

I will describe the structure briefly (to explain why it was tough to determine the root cause):

MyDeviceICD.h - a file that describes the device interface

IGeneralMgr.h - a library interface of a manager class that any manager must inherit but is managed by the framework and not by the application developers.

IMyDeviceMgr.h/cpp - a class that describes the manager interface and inherits the IGeneralMgr.h

CMyDeviceMgr.h/cpp - a class that publicly inherits the IMyDeviceMgr interface class and implements the common functionality for such a device.

CMySpecificVendorDeviceMgr.h/cpp - a class that publicly inherits the CMyDeviceMgr interface class and implements the specific functionality for such a device.

The crash appeared when the managers-factory (another class) tried to create a new object of the CMySpecificVendorDeviceMgr class using the information provided by a configuration file.

After investigating every data member in the chain and how it is initialized, and since no additional useful input was provided by the GDB backtrace, I had to re-apply the problematic commit step by step until I found the root cause.

I am sharing this here since this is where I reached on several online searches of this issue and where others who might have made the same mistake as my team, will have some guidance to check the #pragma as well.

Elielia