Heap Overflow attack, what can go wrong with this code

Question

char *test(char *arg1, char* arg2){
size_t length=strlen(arg1);
char *c= malloc(length+4);
for(int i=length;i>0;i--)
   *(c+i+4)=*(arg1)^(arg2[i%8]);
*(size_t *) (c) =length;
return c;
}

Does this code suffer from heap overflow attack ?

Answer 1

Lots of things can go wrong there. Most importantly, the expression *(c+i+4)=*(arg1)^(arg2[i%8]) is going to overflow your allocated buffer on the first iteration of the loop.

Imagine that length==1. So you'll allocate 5 bytes for c. The first time through the loop, i is equal to 1. So the expression c+i+4 resolves to c+5, which is one byte beyond the memory you allocated.

Other things that can go wrong:

• arg1 is an invalid pointer. Your program crashes.
• The string referenced by arg1 is really long, and you can't allocate enough memory for it. malloc fails and your program crashes.
• Memory addressed by arg2 is smaller than 8 bytes, and therefore your code is reading beyond the allocated memory. This might not crash, but the result will be ... undefined.
• You assume that size_t is 4 bytes. Your malloc should be malloc(length+sizeof(size_t)).