0

I am trying to create a small library for stream AES encryption, I started my work based on Facebook Conceal project (https://github.com/facebook/conceal), just changing some things and improving the wrapper around the native to support ciphers with padding.

It is working and it can decipher files without problems but I get random Heap Memory Corruptions when I work with large streams, and after a lot of time debugging I have been unable to find the error.

Here is my code:
https://gist.github.com/frisco82/9782725

I have tried to find memory allocation or free problems but there are almost no malloc or free, and jni call should be safe, the same goes for openssl ones (I have compiled my own but conceal provided ones also fail)

CheckJni does not warn about anything and while the context handling is a bit out of the box it doesn't seem broken (indeed Android conscrypt seems to use something similar).

Also if someone can point me to a Android native AES multistep (multiple update calls) library I will switch to that and forget this.

The error varies from time to time but it is usually similar to his:

03-26 10:33:02.065: A/dalvikvm(2475): @@@ ABORTING: DALVIK: HEAP MEMORY CORRUPTION IN mspace_malloc addr=0x0
03-26 10:33:02.065: A/libc(2475): Fatal signal 11 (SIGSEGV) at 0xdeadbaad (code=1), thread 2494 (AsyncTask #1)
03-26 10:33:02.205: I/DEBUG(933): *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
03-26 10:33:02.205: I/DEBUG(933): Build fingerprint: 'generic_x86/google_sdk_x86/generic_x86:4.4.2/KK/999428:eng/test-keys'
03-26 10:33:02.205: I/DEBUG(933): Revision: '0'
03-26 10:33:02.205: I/DEBUG(933): pid: 2475, tid: 2494, name: AsyncTask #1  >>> com.proton <<<
03-26 10:33:02.205: I/DEBUG(933): signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr deadbaad

Full stack traces:
http://pastebin.com/f6mDuQEj

frisco
  • 1,897
  • 2
  • 21
  • 29
  • Where's the rest of the crash dump? You need to examine the stack trace in comparison to your program in order to understand the failure. – Chris Stratton Mar 26 '14 at 15:09
  • Added a pastie with a couple of traces, but as this is a memory corruption the strack trace does not show the error, just when the corruption is found, that's why this is a lot more complex. – frisco Mar 26 '14 at 15:24
  • Just curious, why does conceal not fit the bill for you? If you file a issue on github I can possibly help. – user868459 Mar 27 '14 at 21:08
  • @user868459 Well I need to use AES CBC PKCS5 with support for multiple key lengths, as our android app is just one of several apps for diferent platforms and the ciphered files cand and will be shared between them. I will release my changes today as finally worked around the issue. – frisco Mar 28 '14 at 11:46
  • @frisco I see. Would appreciate if you could file a report with your specific use case and workaround. This will help us decide what kind of support Conceal could provide to possibly support that case as well: https://github.com/facebook/conceal/issues?state=open. – user868459 Mar 29 '14 at 16:35

2 Answers2

1

It is working and it can decipher files without problems but I get random Heap Memory Corruptions when I work with large streams.

From above line it looks to me that your program is clearly overwriting the memory which was allocated implicitly or explicitly by your code. I was trying to understand your code however it was not clear to me. But I tried to look from memory corruption scenario and found that your program does have malloc/free call which might lead to memory overrun.

EVP_CIPHER_CTX *ctx = (EVP_CIPHER_CTX*) malloc(sizeof(EVP_CIPHER_CTX));
EVP_CIPHER_CTX_init(ctx);

EVP_CIPHER_CTX *ctx = (EVP_CIPHER_CTX*) malloc(sizeof(EVP_CIPHER_CTX));
EVP_CIPHER_CTX_init(ctx);

I tried to check the layout of the EVP_CIPHER_CTX structure but it was not available in your code. But I saw that these pointers are getting used in various context within your program. Now you should check that under which scenario your buffer can be overwritten as some places you have used different keyLength and depending on this your program is executing different function. I think you may want to review these codes and see whether overflow is possible!!!....

As your application would be running on android based system where we can not run any dynamic tool(Valgrind/WinDBG/Pageheap..) so I guess you need to review your code by putting some log at important place and see where you are overwriting.

Hope above information would be useful for you to understand your problem.

Mantosh Kumar
  • 5,659
  • 3
  • 24
  • 48
  • EVP_CIPHER_CTX and all the EVP callls are calls to openssl library, indeed I tested without that malloc using EVP_CIPHER_CTX_new() that does the malloc internally but no luck. I know the code is not very legible without prior openssl knowledge but it is just a interface for init, update and final functions of the openssl lib. – frisco Mar 26 '14 at 14:23
  • @frisco:Well my explanation was just to give some input so that you may proceed as the code is not easy to understand.You have mentioned that you have already tried it and ruled it out.Well I was not able to figure out anything extra so nothing to add on what i have mentioned in my earlier response. – Mantosh Kumar Mar 26 '14 at 16:07
  • No, don't worry I haven't ruled out anything, I have tried a lot and I am starting to doubt about everything :D I appreciate your comment – frisco Mar 26 '14 at 16:21
1

After all I was able to work around this problem, EVP_CipherUpdate (or jni ReleaseByteArrayElements) sometimes overflow the output buffer causing the heap corruption, nothing in my code was wrong and also it was not a problem with the caller as replacing EVP_CipherUpdate with a memcpy call with the same parameters worked as expected and there was no heap corruption.

So the solution was adding some extra length to the output buffer sent to nativeUpdate and the error was gone.

I have made the full working version of the library for others to use at: https://github.com/frisco82/conceal

frisco
  • 1,897
  • 2
  • 21
  • 29