Skip to content
This repository has been archived by the owner on Sep 25, 2023. It is now read-only.

Known "Bugs"

Zhang edited this page Apr 21, 2019 · 18 revisions

Every single bug listed below is directly extracted from @LeadroyaL 's blog

My implementation is far from perfect and I'm more than happy to discuss such issues, but calling someone else's DOCUMENTED FEATURE a "very stupid bug" multiple times while can't even provide any reasonable improvement himself is just beyond me

Strings are copied on a per-function basis so the following code won't work:

#include<stdio.h>
char hello[] = "HelloWorld!\n";
void repeat(){
printf(hello);
}
int main(){
hello[0] = 'A';
printf(hello);
repeat();
return 0;
}

This "very stupid bug"(direct quote from his blog) is documented since the very beginning. The reason is that not many people will write code like that, and this kind of implementation helps to prevent cases that one dump of the decrypted string broke the encryption on all global references.

Flags in FunctionCallObfuscate uses hard-coded values from host platform and could be wrong in cross-platform environments and could break on non-Apple platforms:

This is by-design because most users of this project are only targeting Apple platform and it's probably stupid to ask for the value from naive users. In the private version, this valve is calculated on-the-fly depending on target triple.

Parallel Access to StringEncrypted function results in racing condition:

Also documented since the very beginning.
Our guy also purposed a "solution" using pthread APIs. Below is his purposed fix.

bool isDecrypt = 0;
pthread_mutex_t lock;
 
__attribute__ ((constructor(101)))
void before(){
    pthread_mutex_init(&lock, 0);
}
 
int func(){
    if(!isDecrypt){
        pthread_mutex_lock(&lock);
        if(!isDecrypt){
            xor_decrypt(data);
            isDecrypt = 1;
        }
        pthread_mutex_unlock(&lock);
        pthread_mutex_destroy(&lock);
    }
    printf(data);
}

I wonder why nobody has ever thought of this implementation. Oh wait, I did! Why I didn't do this?

  • Constructing the type for pthread_mutex_t could be very troublesome and error-prone due to many reasons.
  • By simply analyzing the assembly, the attacker could obtain a one-to-one mapping for the flag and its corresponding function. Then he/she could just hook pthread_mutex_unlock and dump out the decrypted strings, then he/she could just write dumped strings back and nop the comparison part, essentially bypassing the whole protection. Genius move!
  • Your pthread_mutex_lock should be placed at the very beginning otherwise the comparison might leads to racing condition too afaik. This leads us to the next issue.
  • Since the function needs to do lock/unlock on every entry, blindly implementing this could introduce another heavy performance overhead on functions that don't need to be thread-safe. Thus I documented this behaviour and asks the user to manage their own thread safety structure.
  • Since the function needs to do lock/unlock on every entry, you really shouldn't be calling pthread_mutex_destroy for obvious reasons.
  • Your __attribute__ ((constructor(101))) raises other issues like priority scheduling and the possibility that some backend/target doesn't support this kind of intrinsic. Instead, use pthread_mutex_t lock=PTHREAD_MUTEX_INITIALIZER; for proper static initialize. Just do me a favor and stop copy-pasting code snippets you found online.

Cross-TU String Reference is handled incorrectly:

The following two code snippets won't compile due to original string wiped:

#include <stdio.h>

extern char c[10];
int main() {
    printf("c=%s\n", c);
}
#include <stdio.h>
 
char c[10] = {'1', '2', '3', '4', '5'};

Also documented since the very beginning.

StringEncryption inserts junk code even if the function contains no string references

This is probably the only actual bug, congratulations! Although it should be apparent that the actual impact of this bug is approximately zero.

EDIT: Just so you know, all these StringEncryption "bugs" are fully documented (maybe except the last one) and there are methods to disable/enable them on a function-by-function basis. Imagine someone actually read the documentation before asking questions. Very surprising and unexpected, I know.