Challenge 1

Challenge1.png

bug: validate Command input for NULL
fix:

if (Command == NULL)
{
   return false;
}

bug: strncmp of "DoSomething" does not compare null-terminated character of command
fix: use compiler generated size

strncmp(Command, "DoSomething", sizeof("DoSomething"))

#define COMMAND_SOMETHING "DoSomething"
strncmp(Command, COMMAND_SOMETHING, sizeof(COMMAND_SOMETHING));

bug: strncpy does not validate parameter has over 11 characters
fix:

uint32_t length = strlen(Command) + 1;
if (length > sizeof(COMMAND_SOMETHING))
{
   strncpy...
   printf...
}

bug: strlen(Command) computes incorrect sized buffer
fix:

uint32_t data_length = strlen(Command + length) + 1; // <-- Correct strlen

bug: strncpy size needs to use minimum size of data buffer and input data
fix:

uint32_t copy_length = min(data_length, sizeof(data))
strncpy(..., ..., copy_length);

vulnerabilty: stack_overflow caused by overflowing stack's command data

trigger:

Command input size buffer is greater than 1024 + sizeof("DoSomething") = 1036
Attacker controlled data begins at:

Command[1036]
Might be able to overwrite return address to control flow

Challenge 2

Challenge2.png


bug: Time of Check, Time of Use (TOC-TOU) of workers variable
fix: place workers operations in lock

lock()
   workers++;
   ...
   workers--;

   unlock();

bug: operations while holding lock *could* raise an exception

fix:

   lock()
   
   try {

      //do work

   } finally {

      unlock()
   }

Challenge 3

Challenge3.png

bug: validate input parameter

fix:

if (Data == NULL)
{
   return NULL;
}

bug: access violation

fix: wrap all conditions in buffer success check

if (buffer != NULL)
{
   ...
}

bug: double-free AND use-after free in strncmp

fix: nest statements, or keep state

if (length check)
{
   if (value check)
   {
      passed = true;
   }
}
if (!passed)
{
   free
}

bug: use-after free in return

fix: after freeing, always clear pointer

free(buffer);
buffer = NULL;

Challenge 4

Challenge4_a.png
 

1. if at line 72 needs braces or line 74 will execute unconditionally

2. switch case at line 58 needs breaks or the execution will fall through and do more than intended

3. dir on line 54 will always be 0 due to narrowing the uint16 on the right hand side (Need to shift result down)

4. memcpy on line 73 is undefined behavior due to overlapping source and dest buffers (always true since header alone is > 4 bytes)

5. Calculation of the total pkt length on line 69 is wrong.  sizeof(hdr) will return the size of the pointer, not the struct pointed to. 

Could lead to a buffer overflow if max_len is too small

6. If statement on line 72 is missing { } braces.

Challenge 5

Challenge5.png

1. number of samples to read on line 10 need to be capped at MAX_SAMPLES entering more than MAX_SAMPLES arguments will overflow the samples buffer

2. strtol on line 14 returns a long int - possible loss of precision and unintended value when if it wraps into a int32_t (This is architecture dependent.  long int is at least 4 bytes but could be 8 like on osx/linux 64 bit systems (windows it is only 4)) 

3. condition in if on line 23 should have == not =.  As is it will always be true since an assignment returns the value of the left hand side after the assignment and 100 != 0 => true

4. Loop on line 30 is an infinite loop due to the unsigned type.  Instead of going to -1 and ending it will wrap to 2**32-1 and cause bad things to happen

5. Line 19 should be adding samples[i];

6. Line 17: sum is not initialized to 0.