Double free or corruption crash

Question

I have a program (smbanner) that reads/writes from a database, and recently I've added a cryptography routine to encrypt the data on DB-write and decrypt it on DB-read (smbanner does the reading/writing by calling some DB read/write routines elsewhere... in those DB routines, I've injected my cryptography calls). I'll also add that I got this thrown onto my lap, and C is nowhere near my first language, so please be kind!

All my code compiles fine, cppcheck static analysis says it's fine, and multiple environments run it just fine... except one. On that environment, I get the following:

[root@mndemo update]# *** glibc detected *** /home/silentm/bin/smbanner: double free or corruption (!prev): 0x000000000455e750 ***
======= Backtrace: =========
/lib64/libc.so.6[0x374e075e66]
/lib64/libc.so.6[0x374e0789b3]
/lib64/libc.so.6(fclose+0x14d)[0x374e0664cd]
/home/silentm/bin/smbanner[0x4b6ffb]
/home/silentm/bin/smbanner[0x480f00]
/home/silentm/bin/smbanner[0x40dcac]
/lib64/libc.so.6(__libc_start_main+0xfd)[0x374e01ed5d]
/home/silentm/bin/smbanner[0x403849]
======= Memory map: ========
00400000-00613000 r-xp 00000000 09:01 25886937                           /home/silentm/bin/smbanner
00813000-008e7000 rw-p 00213000 09:01 25886937                           /home/silentm/bin/smbanner
008e7000-00b11000 rw-p 00000000 00:00 0 
01091000-0456f000 rw-p 00000000 00:00 0                                  [heap]
3569000000-35691b9000 r-xp 00000000 09:00 917372                         /usr/lib64/libcrypto.so.1.0.1e
35691b9000-35693b8000 ---p 001b9000 09:00 917372                         /usr/lib64/libcrypto.so.1.0.1e
35693b8000-35693d3000 r--p 001b8000 09:00 917372                         /usr/lib64/libcrypto.so.1.0.1e
35693d3000-35693df000 rw-p 001d3000 09:00 917372                         /usr/lib64/libcrypto.so.1.0.1e
35693df000-35693e3000 rw-p 00000000 00:00 0 
374dc00000-374dc20000 r-xp 00000000 09:00 783364                         /lib64/ld-2.12.so
374de1f000-374de20000 r--p 0001f000 09:00 783364                         /lib64/ld-2.12.so
374de20000-374de21000 rw-p 00020000 09:00 783364                         /lib64/ld-2.12.so
374de21000-374de22000 rw-p 00000000 00:00 0 
374e000000-374e18a000 r-xp 00000000 09:00 783367                         /lib64/libc-2.12.so
374e18a000-374e38a000 ---p 0018a000 09:00 783367                         /lib64/libc-2.12.so
374e38a000-374e38e000 r--p 0018a000 09:00 783367                         /lib64/libc-2.12.so
374e38e000-374e38f000 rw-p 0018e000 09:00 783367                         /lib64/libc-2.12.so
374e38f000-374e394000 rw-p 00000000 00:00 0 
374e400000-374e417000 r-xp 00000000 09:00 783373                         /lib64/libpthread-2.12.so
374e417000-374e617000 ---p 00017000 09:00 783373                         /lib64/libpthread-2.12.so
374e617000-374e618000 r--p 00017000 09:00 783373                         /lib64/libpthread-2.12.so
374e618000-374e619000 rw-p 00018000 09:00 783373                         /lib64/libpthread-2.12.so
374e619000-374e61d000 rw-p 00000000 00:00 0 
374e800000-374e802000 r-xp 00000000 09:00 783380                         /lib64/libdl-2.12.so
374e802000-374ea02000 ---p 00002000 09:00 783380                         /lib64/libdl-2.12.so
374ea02000-374ea03000 r--p 00002000 09:00 783380                         /lib64/libdl-2.12.so
374ea03000-374ea04000 rw-p 00003000 09:00 783380                         /lib64/libdl-2.12.so
374ec00000-374ec15000 r-xp 00000000 09:00 783377                         /lib64/libz.so.1.2.3
374ec15000-374ee14000 ---p 00015000 09:00 783377                         /lib64/libz.so.1.2.3
374ee14000-374ee15000 r--p 00014000 09:00 783377                         /lib64/libz.so.1.2.3
374ee15000-374ee16000 rw-p 00015000 09:00 783377                         /lib64/libz.so.1.2.3
374f400000-374f483000 r-xp 00000000 09:00 783388                         /lib64/libm-2.12.so
374f483000-374f682000 ---p 00083000 09:00 783388                         /lib64/libm-2.12.so
374f682000-374f683000 r--p 00082000 09:00 783388                         /lib64/libm-2.12.so
374f683000-374f684000 rw-p 00083000 09:00 783388                         /lib64/libm-2.12.so
3750800000-3750816000 r-xp 00000000 09:00 783389                         /lib64/libgcc_s-4.4.7-20120601.so.1
3750816000-3750a15000 ---p 00016000 09:00 783389                         /lib64/libgcc_s-4.4.7-20120601.so.1
3750a15000-3750a16000 rw-p 00015000 09:00 783389                         /lib64/libgcc_s-4.4.7-20120601.so.1
3757c00000-3757d49000 r-xp 00000000 09:00 929936                         /usr/lib64/libxml2.so.2.7.6
3757d49000-3757f48000 ---p 00149000 09:00 929936                         /usr/lib64/libxml2.so.2.7.6
3757f48000-3757f51000 rw-p 00148000 09:00 929936                         /usr/lib64/libxml2.so.2.7.6
3757f51000-3757f53000 rw-p 00000000 00:00 0 
7f657423e000-7f657424a000 r-xp 00000000 09:00 783433                     /lib64/libnss_files-2.12.so
7f657424a000-7f657444a000 ---p 0000c000 09:00 783433                     /lib64/libnss_files-2.12.so
7f657444a000-7f657444b000 r--p 0000c000 09:00 783433                     /lib64/libnss_files-2.12.so
7f657444b000-7f657444c000 rw-p 0000d000 09:00 783433                     /lib64/libnss_files-2.12.so
7f657444c000-7f6574451000 rw-p 00000000 00:00 0 
7f6574465000-7f6574467000 rw-p 00000000 00:00 0 
7fff289a1000-7fff28bcf000 rw-p 00000000 00:00 0                          [stack]
7fff28bff000-7fff28c00000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]

The database routines to read/write are as follows (I've omitted the irrelevant code in these functions - represented by ellipses):

ENTRY sfuint
bt3DataRead _FL((fd, recno, data))
ISFILE *fd _DL
sflong recno _DL
void *data _EL
{
  ...
  lioRead(fd -> fdData, recno, trueBase(fd -> lastData))
  decrypt_db_rawData(fd -> lastData, fd->reclen - BASEOFFSET); //this routine is intelligent enough to know whether to actually decrypt or not, so it should be safe to call it here regardless of whether data is actually encrypted or not
  ...
}

ENTRY sfuint
bt3Write _FL((fd, data))
ISFILE *fd _DL
void *data _EL
{
  ...
  void *dataCopy = malloc(fd->reclen);                    //for saving a copy of the data, in its non-encrypted form, so later logic will work
  memcpy(trueBase(dataCopy), trueBase(data), fd->reclen); //copy aside the raw unencrypted data for safe-keeping while we encrypt, next
  encrypt_db_rawData(data, fd->reclen-BASEOFFSET);        //encrypt the data
  lioWrite(fd -> fdData, recno, trueBase(data));          //write encrypted data to database
  memcpy(trueBase(data), trueBase(dataCopy), fd->reclen); //now that the write is complete, restore the non-encrypted data, so stuff works right later
  ...
}

The cryptography routines are as follows (the data is just an array of bytes technically; but strings, in practice). They iterate through each byte in the array, and do crypto on each one at a time:

int encrypt_db_rawData(char *data, size_t data_size) {
  int ret = 0;
  char *buf_out;                   //where the whole encrypted byte "string" gets assembled
  char obuf;                       //declare our per-byte output buffer
  int ilen, olen;                  //stuff needed by EVP_CipherUpdate
  EVP_CIPHER_CTX ctx;              //declare our EVP cipher context
  int currbyte

  /* ... setup cipher context for encryption (omitted) ... */

  buf_out = malloc(data_size);
  memset(buf_out, 0x00, data_size); //initialize buffer

  for(currbyte = 0; currbyte <= data_size - 1; currbyte++) {
    ret = EVP_CipherUpdate(&ctx, &obuf, &olen, &data[currbyte], 1); //actually do the encryption, outputting the current data byte to obuf as an encrypted byte
    if(ret != 1) {
      //encryption failed, so abort the entire function (no data modified)
      free(buf_out);
      return ret;
    }
    buf_out[currbyte] = obuf; //encryption of this byte succeeded, so append the encrypted byte to the main output buffer (which may be saved back into *data when all is done)
  }

  memcpy((void*)data, buf_out, data_size); //NOTE: not sure if we really need to cast as a void*, but doesn't seem to hurt (since the calling routine uses void*data instead of char*data)
  free(buf_out);
  return ret;
}

int decrypt_db_rawData(char *data, size_t data_size) {
  int ret = 0;
  char *buf_out;                   //where the whole decrypted byte "string" gets assembled
  char obuf;                       //declare our per-byte output buffer
  int ilen, olen;                  //stuff needed by EVP_CipherUpdate
  EVP_CIPHER_CTX ctx;              //declare our EVP cipher context
  int currbyte;

  /* ... setup cipher context for decryption (omitted) ... */

  buf_out = malloc(data_size);
  memset(buf_out, 0x00, data_size); //initialize buffer

  for(currbyte = 0; currbyte <= data_size - 1; currbyte++) {
    ret = EVP_CipherUpdate(&ctx, &obuf, &olen, &data[currbyte], 1); //actually do the decryption, outputting the current data byte to obuf as an decrypted byte
    if(ret != 1) {
      //decryption failed, so abort the entire function (no data modified)
      free(buf_out);
      return ret;
    }
    buf_out[currbyte] = obuf; //decryption of this byte succeeded, so append the decrypted byte to the main output buffer (which may be saved back into *data when all is done)
  }

  memcpy((void*)data, buf_out, data_size); //NOTE: not sure if we really need to cast as a void*, but doesn't seem to hurt (since the calling routine uses void*data instead of char*data)
  free(buf_out);
  return ret;
}

Answer 1

The issue was in the byte/pointer shifting being done by trueBase... was probably overlapping some memory space in my memcpy operation. Valgrind was essential to quickly finding this out! Thanks to @lrleon for that.

Here is my updated code in the DB write routine:

ENTRY sfuint
bt3Write _FL((fd, data))
ISFILE *fd _DL
void *data _EL
{
  ...
  void *dataCopy = malloc(fd->reclen - BASEOFFSET);          //for saving a copy of the data, in its non-encrypted form, so later logic will work
  memcpy(dataCopy, trueBase(data), fd->reclen - BASEOFFSET); //copy aside the raw unencrypted data for safe-keeping while we encrypt, next
  encrypt_db_rawData(data, fd->reclen-BASEOFFSET);           //encrypt the data
  lioWrite(fd -> fdData, recno, trueBase(data));             //write encrypted data to database
  memcpy(trueBase(data), dataCopy, fd->reclen - BASEOFFSET); //now that the write is complete, restore the non-encrypted data, so stuff works right later
  if(dataCopy)
    free(dataCopy);
  ...
}