New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global buffer overflow due to incorrect protocol id in codec_data #27

Closed
bshastry opened this Issue May 3, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@bshastry

bshastry commented May 3, 2017

Hi,

There's a potential global buffer overflow when handling a packet that I have called global_buffer_overflow.pcap that corrupts the value of codec_data.next_prot_id to -1 in turn resulting in a buffer overread at
208 mapped_prot = CodecManager::s_proto_map[to_utype(codec_data.next_prot_id)];

in packet_manager.cc:208

Here's the full stack trace:

#0  0x00007ffff576e428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007ffff577002a in __GI_abort () at abort.c:89
#2  0x000000000052f2a9 in __sanitizer::Abort() ()
#3  0x000000000051c109 in __asan::AsanDie() ()
#4  0x0000000000523162 in __sanitizer::Die() ()
#5  0x000000000051b086 in __asan_report_error ()
#6  0x000000000051c4b3 in __asan_report_load1 ()
#7  0x00000000009dec10 in PacketManager::decode (p=0x610000017f40, pkthdr=0x7fffef7d5a20, pkt=0x631000258800 "\377\355\001\001\001\001\0
01", cooked=false) at packet_manager.cc:208
#8  0x000000000059c0c8 in Snort::process_packet (p=0x610000017f40, pkthdr=0x7fffef7d5a20, pkt=0x631000258800 "\377\355\001\001\001\001\0
01", is_frag=false) at snort.cc:772
#9  0x000000000059c856 in Snort::packet_callback (pkthdr=0x7fffef7d5a20, pkt=0x631000258800 "\377\355\001\001\001\001\001") at snort.cc:
883
#10 0x0000000000a15edd in pcap_process_loop (user=0x61500002fb00 "", pkth=<optimized out>, data=<optimized out>) at daq_pcap.c:370
#11 0x00007ffff6c40ac4 in ?? () from /usr/lib/x86_64-linux-gnu/libpcap.so.0.8
#12 0x0000000000a1594d in pcap_daq_acquire (handle=0x61500002fb00, cnt=0, callback=<optimized out>, metaback=<optimized out>, user=<opti
mized out>) at daq_pcap.c:388
#13 0x00000000009ad262 in SFDAQInstance::acquire (this=0x60e000006f60, max=0, callback=0x59c632 <Snort::packet_callback(void*, _daq_pkth
dr const*, unsigned char const*)>) at sfdaq.cc:487
#14 0x000000000058b56e in Analyzer::analyze (this=0x61200000b740) at analyzer.cc:160
#15 0x000000000058b0a6 in Analyzer::operator() (this=0x61200000b740, ps=0x603000013ab0) at analyzer.cc:98
#16 0x000000000054d312 in std::__invoke<Analyzer, Swapper*>(Analyzer&, Swapper*&&) (__f=...) at /usr/include/c++/5/functional:201
#17 0x000000000054d287 in std::reference_wrapper<Analyzer>::operator()<Swapper*>(Swapper*&&) const (this=0x606000010d30) at /usr/include
/c++/5/functional:428
#18 0x000000000054d249 in std::_Bind_simple<std::reference_wrapper<Analyzer> (Swapper*)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) (this=0x606000010d28) at /usr/include/c++/5/functional:1531
#19 0x000000000054d10e in std::_Bind_simple<std::reference_wrapper<Analyzer> (Swapper*)>::operator()() (this=0x606000010d28) at /usr/inc
lude/c++/5/functional:1520
#20 0x000000000054d09e in std::thread::_Impl<std::_Bind_simple<std::reference_wrapper<Analyzer> (Swapper*)> >::_M_run() (this=0x60600001
0d10) at /usr/include/c++/5/thread:115
#21 0x00007ffff62f6c80 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#22 0x00007ffff5d1f6ba in start_thread (arg=0x7fffef8aa700) at pthread_create.c:333
#23 0x00007ffff583f82d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
(gdb) p codec_data
$4 = {next_prot_id = -1, lyr_len = 14, invalid_bytes = 0, proto_bits = 512, codec_flags = 0, ip_layer_cnt = 0 '\000', ip6_extension_count = 0 '\000', curr_ip6_extension = 0 '\000', ip6_csum_proto = IpProtocol::IP}
@snortadmin

This comment has been minimized.

Show comment
Hide comment
@snortadmin

snortadmin May 3, 2017

Collaborator

Thanks. This is due to an off-by-1 array size for the 16-bit proto IDs. Working on a fix.

Collaborator

snortadmin commented May 3, 2017

Thanks. This is due to an off-by-1 array size for the 16-bit proto IDs. Working on a fix.

@snortadmin

This comment has been minimized.

Show comment
Hide comment
@snortadmin

snortadmin May 4, 2017

Collaborator

This is now fixed. The array was sized to accommodate all possible values.

Collaborator

snortadmin commented May 4, 2017

This is now fixed. The array was sized to accommodate all possible values.

@snortadmin snortadmin closed this May 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment