emptymonkey / revsh

A reverse shell with terminal support, data tunneling, and advanced pivoting capabilities.
MIT License
458 stars 90 forks source link

Fix use after free #16

Closed jacob-baines closed 6 years ago

jacob-baines commented 7 years ago

I was investigating compiling a static build with musl and revsh kept binding to random ports.

From io_nossl.c:

free(ip_address);

memset(&name, 0, sizeof(name));
name.sin_family = AF_INET;
name.sin_port = htons(strtol(ip_port, NULL, 10));

In the above, "ip_port" points to memory within ip_address. So bind is being passed data that has been freed. Here is some valgrind output that also points this out:

==115528== Memcheck, a memory error detector
==115528== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==115528== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==115528== Command: ./revsh -vvv -c
==115528== 
==115528== Invalid read of size 1
==115528==    at 0x4E75454: ____strtol_l_internal (strtol_l.c:293)
==115528==    by 0x40313B: init_io_control (in /home/albinolobster/revsh)
==115528==    by 0x403C51: do_control (in /home/albinolobster/revsh)
==115528==    by 0x4024D9: main (in /home/albinolobster/revsh)
==115528==  Address 0x5204268 is 8 bytes inside a block of size 13 free'd
==115528==    at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==115528==    by 0x403118: init_io_control (in /home/albinolobster/revsh)
==115528==    by 0x403C51: do_control (in /home/albinolobster/revsh)
==115528==    by 0x4024D9: main (in /home/albinolobster/revsh)
==115528==  Block was alloc'd at
==115528==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==115528==    by 0x403077: init_io_control (in /home/albinolobster/revsh)
==115528==    by 0x403C51: do_control (in /home/albinolobster/revsh)
==115528==    by 0x4024D9: main (in /home/albinolobster/revsh)
==115528== 
==115528== Invalid read of size 1
==115528==    at 0x4E75654: ____strtol_l_internal (strtol_l.c:433)
==115528==    by 0x40313B: init_io_control (in /home/albinolobster/revsh)
==115528==    by 0x403C51: do_control (in /home/albinolobster/revsh)
==115528==    by 0x4024D9: main (in /home/albinolobster/revsh)
==115528==  Address 0x5204269 is 9 bytes inside a block of size 13 free'd
==115528==    at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==115528==    by 0x403118: init_io_control (in /home/albinolobster/revsh)
==115528==    by 0x403C51: do_control (in /home/albinolobster/revsh)
==115528==    by 0x4024D9: main (in /home/albinolobster/revsh)
==115528==  Block was alloc'd at
==115528==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==115528==    by 0x403077: init_io_control (in /home/albinolobster/revsh)
==115528==    by 0x403C51: do_control (in /home/albinolobster/revsh)
==115528==    by 0x4024D9: main (in /home/albinolobster/revsh)
==115528== 

I just moved the free() until after strtol had been called.

emptymonkey commented 6 years ago

Good catch and sorry it's taken me so long to get back to this. I've merged this in with the devel branch. I'm cleaning stuff up and adding some features, then I'll move everything to the master branch at once.