leaky implementation / thoughts

[stephenmathieson]

i've got a side project which needs to consume tiny packets of data.. i remember this getting put together, so i thought i'd check it out.

it looks like there's a possible buffer overflow in amp_decode_arg().

also, the tests leak every byte of memory they allocate.

here's a valgrind report:

vagrant@precise64:/vagrant/amp$ make test.out && valgrind --leak-check=full ./test.out
cc test.c amp.c -o test.out -std=c99
==8345== Memcheck, a memory error detector
==8345== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==8345== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==8345== Command: ./test.out
==8345== 
==8345== Invalid read of size 1
==8345==    at 0x4006DB: main (in /vagrant/amp/test.out)
==8345==  Address 0x51f10a4 is 0 bytes after a block of size 4 alloc'd
==8345==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8345==    by 0x4008E7: amp_decode_arg (in /vagrant/amp/test.out)
==8345==    by 0x4006A9: main (in /vagrant/amp/test.out)
==8345== 
==8345== Invalid read of size 1
==8345==    at 0x400721: main (in /vagrant/amp/test.out)
==8345==  Address 0x51f10f5 is 0 bytes after a block of size 5 alloc'd
==8345==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8345==    by 0x4008E7: amp_decode_arg (in /vagrant/amp/test.out)
==8345==    by 0x4006A9: main (in /vagrant/amp/test.out)
==8345== 
==8345== Invalid read of size 1
==8345==    at 0x400763: main (in /vagrant/amp/test.out)
==8345==  Address 0x51f1144 is 0 bytes after a block of size 4 alloc'd
==8345==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8345==    by 0x4008E7: amp_decode_arg (in /vagrant/amp/test.out)
==8345==    by 0x4006A9: main (in /vagrant/amp/test.out)
==8345== 
ok
==8345== 
==8345== HEAP SUMMARY:
==8345==     in use at exit: 39 bytes in 4 blocks
==8345==   total heap usage: 4 allocs, 0 frees, 39 bytes allocated
==8345== 
==8345== 13 bytes in 3 blocks are definitely lost in loss record 1 of 2
==8345==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8345==    by 0x4008E7: amp_decode_arg (in /vagrant/amp/test.out)
==8345==    by 0x4006A9: main (in /vagrant/amp/test.out)
==8345== 
==8345== 26 bytes in 1 blocks are definitely lost in loss record 2 of 2
==8345==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8345==    by 0x400A18: amp_encode (in /vagrant/amp/test.out)
==8345==    by 0x400624: main (in /vagrant/amp/test.out)
==8345== 
==8345== LEAK SUMMARY:
==8345==    definitely lost: 39 bytes in 4 blocks
==8345==    indirectly lost: 0 bytes in 0 blocks
==8345==      possibly lost: 0 bytes in 0 blocks
==8345==    still reachable: 0 bytes in 0 blocks
==8345==         suppressed: 0 bytes in 0 blocks
==8345== 
==8345== For counts of detected and suppressed errors, rerun with: -v
==8345== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 2 from 2)

---

unrelated: i thought it'd also be nice if we were able to:

#include "amp.h"

int main(){
  char *args[] = { "some", "stuff", "here" };
  char *buf = amp_encode(args, 3);
  printf("%s\n", buf);
  return 0;
}

and actually see how the packets are structured as plaintext. this way, it'd be trivial to send them via telnet(1) (or whatever).

[tj]

oops, feel free to make changes, I didn't end up using it yet, however changing the "protocol" would make it different than the node version etc so that might as well be a different lib

[Qard]

The memory leak is due to the test.c file not freeing the buf and arg variables. Also, it doesn't look like the code is respecting null terminators. That's probably the source of the "Invalid read of size 1" warnings.