Memory leak in ClearTrait()

The function ClearTrait overwrites trait->bytes_sha256 with NULL. The memory which may have been allocated is not freed resulting in a memory leak.
The same is true for trait->trait.

BTW, why not use a string or vector?

I believe I had issues, with string types performing their deconstructor sooner than I was done with them.

binlex/src/decompiler.cpp

Lines 457 to 468 in 0f84757

    
           void Decompiler::ClearTrait(struct Trait *trait){ 
        
               trait->tmp_bytes.clear(); 
        
               trait->edges = 0; 
        
               trait->instructions = 0; 
        
               trait->blocks = 0; 
        
               trait->offset = 0; 
        
               trait->size = 0; 
        
               trait->invalid_instructions = 0; 
        
               trait->tmp_trait.clear(); 
        
               trait->trait = NULL; 
        
               trait->bytes_sha256 = NULL; 
        
           }

This is then either a bug in the STL or sound more like a use after free to me which could result in a bug exploitable by malicious code.
Worth investigating, though.

 void Decompiler::ClearTrait(struct Trait *trait){ 
     trait->tmp_bytes.clear(); 
     trait->edges = 0; 
     trait->instructions = 0; 
     trait->blocks = 0; 
     trait->offset = 0; 
     trait->size = 0; 
     trait->invalid_instructions = 0; 
     trait->tmp_trait.clear(); 
     trait->trait = NULL; 
     if (trait->bytes_sha256 != NULL){
         free(trait->bytes_sha256);
         trait->bytes_sha256 = NULL;
     }
 }

With the decompiler having some work done on it so far, might be worth trying to make the data a string again and see if I encounter the same issue I did long ago.

Yup, so switched it to type string and the hashes print out in debug when TraitWorker is called, but the do not show up in the json, they show up as a blank string.

Code in #88 fixed this, closed

BTW, why not use a string or vector?

You can't malloc a structure in C++ with non-trivial constructors. When you have a string in a structure and it's just malloc-ed the constructor of the string class is not called. For situations like that a "new" operator is required or object is allocated statically. In both cases the constructor of string is also called, which is not when using malloc. (The difference between classes and structs in C++ is almost non-existent, unlike C)

Since the two members we were looking at were hashes of known and exact sizes, an array of fixed size is more appropriate. A string provides flexibility at a cost, and we don't need that flexibility in this case.

I found a quick chat, if you're interested. I'm sure there's more you can find:
https://stackoverflow.com/questions/31269728/string-in-struct

Hope this helps, cheers.

Ah, when malloc is used this would explain it. As we are using C++ we could just use new and delete. Then it is possible to have proper constructors and destructors.

	void Decompiler::ClearTrait(struct Trait *trait){
	trait->tmp_bytes.clear();
	trait->edges = 0;
	trait->instructions = 0;
	trait->blocks = 0;
	trait->offset = 0;
	trait->size = 0;
	trait->invalid_instructions = 0;
	trait->tmp_trait.clear();
	trait->trait = NULL;
	trait->bytes_sha256 = NULL;
	}