Non-zero exit status 139 due to free() statement for a memory pointer

Question

In the below attached code I am re-encrypting a file by opening, reading the contents into a buffer, re-encrypting the buffer with a new key and then writing back to the file.

The program is being run as a subprocess in a python/django based server. which prints the error output as: *** Error in/home/kunal/Documents/MyCrest/cloud/backend/mainbgw': double free or corruption (!prev): 0x0000000000ecc250 ***`

The last statement free(ciphertext) to clean up the allocated memory gives me error sometimes where the program exits with a status code of 139 i.e double free or corruption If I remove that statement then the code works perfectly, but I would like return from the function the right way by freeing up the allocated memory.

int update_encryption(char *fileName, char *base_k1, char *base_k1_new, const char* privateKey)
{
  FILE *file;
  size_t cipherlen,keylen;
  unsigned char *ciphertext,*k1_temp,*k1_new_temp,*k1,*k1_new;

  //read ciphertext from the file to be updated
  file = fopen(fileName,"rb");  //open in read binary stream mode
  if (file)
  {
    fseek (file, 0, SEEK_END);
    cipherlen = ftell (file);
    fseek (file, 0, SEEK_SET);
    ciphertext = (unsigned char*) malloc(cipherlen*sizeof(unsigned char));
    if (ciphertext)
    {
      fread (ciphertext, sizeof(unsigned char), cipherlen, file);
    }
    fclose (file);
  }

  //decrypt the data
  if(!Base64Decode(base_k1, &k1_temp, &keylen))
  {
    k1 = (unsigned char*)malloc(sizeof(unsigned char)*374);
    keylen = private_decrypt(k1_temp,keylen,(unsigned char *)privateKey, k1);
    k1[keylen]='\0';
    shaCrypt(ciphertext,(int)cipherlen, (const char *)k1, keylen);
    free(k1_temp);
    free(k1);
  }
  else
    return 1;

  //re-encrypt the data
  if(!Base64Decode(base_k1_new,&k1_new_temp,&keylen))
  {
    k1_new = (unsigned char*)malloc(sizeof(unsigned char)*374);
    keylen = private_decrypt(k1_temp,keylen,(unsigned char *)privateKey, k1_new);
    k1_new[keylen]='\0';
    shaCrypt(ciphertext,(int)cipherlen,(const char*)k1_new, keylen);
    free(k1_new_temp);
    free(k1_new);
  }
  else
    return 1;

  //write the encrypted data to file
  file = fopen(fileName,"wb");
  if (file)
  {
    fwrite(ciphertext, sizeof(unsigned char), cipherlen, file);
    fclose(file);
  }
  else
    return 1;

  //free memory for ciphertext
  if(ciphertext)
    free(ciphertext);
  return 0;
}

EDIT: The error occurs only for files with size 3kB or more, since 139 is the error code for memory corruption OR double free, I guess it is the former case as there is nowhere where I am freeing a memory location twice in my code.

FWIW, you don't need the check `if(ciphertext)` before `free()`ing. It's valid to pass a NULL pointer to `free()`. :-) — Sourav Ghosh, Jul 24 '15 at 07:52
You have `if(ciphertext)` but `ciphertext` is not initialized to 0, so if the first `if` block isn't entered, you're going to pass an invalid pointer to free. — nemetroid, Jul 24 '15 at 07:54
Also, I see you did check for `maloc()` success, good, but what is it failed? That case should be handled first.. — Sourav Ghosh, Jul 24 '15 at 07:54
Yeah I know, the if statement has got nothing to do with that, I had just put it as a check for, in case the malloc fails but nope the problem is not due to that — bawejakunal, Jul 24 '15 at 07:55
@SouravGhosh, the malloc isn't failing though, the file is being written correctly as expected — bawejakunal, Jul 24 '15 at 07:56
@bawejakunal [Don't be so sure](https://en.wikipedia.org/wiki/Undefined_behavior). — Sourav Ghosh, Jul 24 '15 at 07:58
I can spot memory leak, but not double free in the code. There are 3 "return 1" that could cause the function exit without releasing ciphertext. — simon, Jul 24 '15 at 08:02
If the file can't be opened properly, `ciphertext` will never be allocated. Since it's also not set to NULL, it will point to a random memory location and `free(ciphertext)` will crash. — , Jul 24 '15 at 08:07
And as always, [please don't cast the result of `malloc()`](http://stackoverflow.com/a/605858/3233393). — Quentin, Jul 24 '15 at 08:21
Could you provide a main function that uses the above function with just enough data that it reproduces the crash? I.e., a minimal self-contained example. — , Jul 24 '15 at 08:25
@simon thanks for pointing those out, fixed them now but the problem still persists, I added NULL as well as as evert suggested but no help on that end either, the program still exits with 139 or sometimes 134 — bawejakunal, Jul 24 '15 at 08:28
@Evert yeah I did include stdlib else nothing would have worked :-/ — bawejakunal, Jul 24 '15 at 08:29
What is the signature of shaCrpt? it looks suspicious to me. Does it change the ciphertext by any chance? — simon, Jul 24 '15 at 08:33
@simon it's part of a ongoing work so sorry I can't post a lot of code on the public forum, shaCrypt is a function to xor the bytes of ciphertext in chunks of 20 bytes with different SHA1 digests, it modifies the ciphertext string in place so no malloc or free in that function. — bawejakunal, Jul 24 '15 at 08:50
A simple test will be to comment out or mock the function body of shaCript to see if you error still remain. — simon, Jul 24 '15 at 09:50

score 2 · Answer 1 · answered Jul 24 '15 at 08:08

2

If the file can't be opened properly, ciphertext will never be allocated. Since it's also not set to NULL, it will point to a random memory location and free(ciphertext) will crash.

Set ciphertext = NULL near you declarations.

answered Jul 24 '15 at 08:08

That changed the exit status from 139 to 134. – bawejakunal Jul 24 '15 at 08:20
134 could be "assertion failed". Does it come with an error message? – Jul 24 '15 at 08:24
It is failing for file sizes above 3kB with error codes 139, do you think that could be causing memory overflow or something like that since 139 is for double free OR memory corruption ? – bawejakunal Jul 24 '15 at 09:01

Non-zero exit status 139 due to free() statement for a memory pointer

1 Answers1