We're given the ability to select an index to leave a message on, a length, and a message. Often times if you can select an index, it might open up opportunities for arbitrary writes. Trying different numbers here is usually a good strategy. I like to try 0 (which is usually a good working case), some large number (can i write to a place far away?), some negative number (can i write to a place likely i shouldn't?).
Segmentation fault (core dumped)
Ok, so clearly it's not doing bounds checks there. What about negative small?
Index too large
So something is strange already. It's treating -1 as an unsigned int (thus too large), but it's OK attempting to dereference my 65535? Let's look at size.
message too long, you can leave on memo though
Looks like there's some difference when using a large length. Let's dig into the code a bit here. Note, I'm using IDA in this case as radare2 has issues parsing the jump table correctly.
We can see why the large index was causing a segfault. In the very first block, this binary prompts for an integer, then uses that in an offset to a static global location. It's interesting to note that the global Index is set prior to any sanity checking. Further, the compare in the second block is a "ja" or "jump above", which is an unsigned comparison operator.
We also noted that with a large value for the length we received a message about it being too long. We can see that on the left. Let's take a closer look:
First, the length is compared against 32 (0x20), and the branch is chosen based on that.
Length <= 32
- print message
- read size bytes into just malloc'd space
- Load offset into global array of memo pointers
- Save pointer to malloc'd space into pointer array
- Compute another offset into the memo pointers array and store the stack variable address in it (!?)
- Store the length of the memo into a global memo length array
Note for a second step 6. Consider why this would be unusual. The answer is that you're storing a stack address to a global variable. Remember, the stack is meant to be temporary for that procedure and only contain local variables. There's really no good reason to store a pointer to the local variable in a global variable. Once you exit the function the reference will be moot and the context will be lost. From an exploiter perspective, this looks like a target to leak the stack address.
Length > 32
- print message
- malloc(32) (?!)
- read in size byte into previously malloc'd space (?!)
Couple strange things here. First off, you just told the program that you would give it more than 32 bytes of data, and it only allocated 32 bytes. So there's an obvious heap overflow here. Further, it doesn't bother storing the address of your buffer anywhere globally or returning it. This is actually a memory leak in the conventional sense that you have no pointers in your code to this area in memory after the function returns.
This turned out to be one of the "correct" attack vectors. However, it involves malloc trick that I'm not as versed in. Read other writeups for that version. :-)
What can we say about this function now? Well, it give us the primitive to set a global variable iIndex. It loads an address before bounds checking. On small sizes of length, it provides us the ability to leak the stack address by storing it at a global address. Finally, it provides us a primitive for an arbitrarily large sized malloc (generally a bad idea), and a heap overwrite.
That's probably enough for one function. Let's move on.