Couple of minor issues in command handling code

[JohnLaTwC]

Issue 1

Check for failed allocation to avoid access violation on write

    // Make sure we have enough space to add '\0' character at end.
    enc = (char*)b64_realloc(enc, size + 1);
! Should check for failed allocation before the next assignment.
-    enc[size] = '\0';
+    if (NULL != enc) {
+        enc[size] = '\0';
+    } else {
+        return NULL;
+    }

trevorc2/agents/c/src/encode.c

Line 89 in cb508ca

enc = (char*)b64_realloc(enc, size + 1);

Issue 2

I am not sure of the intent when doTasking is on the nth iteration of getting shell command output but fails due memory allocation. Currently the code returns the buffer with as much as it was able to read. But there is some unreachable code that may be a remnant of treating this as an error case.

unsigned char* doTasking(unsigned char* command, int* outSize)
...

    while (1) {
        tempshelldata = realloc(shelldata, fullReadSize + 2048 + 1);
        if (tempshelldata == NULL) {
            break;
! breaks out of the loop when realloc returns NULL
        }
        if (tempshelldata == NULL) {
! this code is not reachable because the check against NULL above breaks out of the loop
! if this code was reached, it does not set `shelldata` to NULL after `free` which would result in callers to `doTasking` 
! operating on undefined memory and potentially a double-free because they also call `free`
            free(shelldata);
        }

trevorc2/agents/c/src/main.c

Line 326 in 4154bd2

if (tempshelldata == NULL) {

[HackingDave]

Thanks for the report, merged and fixed.

[JohnLaTwC]

My only other comment is that when shelldata is set to NULL in that PR, you may also want to set fullReadSize = 0 Otherwise you can end up with a case where if realloc fails during the Nth iteration of a loop the function will return NULL but with a non-zero length in the outSize param. The main function does not check for a NULL return from doTasking and passes the output values to sendTasking. A quick check of sendTasking didn't see an obvious problem due to a difference in length between a NULL shelldata with an incorrect length, but it may be safer to check for NULL before calling sendTasking and ensuring length returned from doTasking is zero if it returns NULL.