[Bro-Dev] File Analysis Inconsistencies

[Aaron Eppert]

Justin,

Thank you. I peeled the egg off my face and updated the github code
accordingly.

However, I have run into an additional interesting tidbit if I use event
file_sniff to attach an analyzer or Files::register_for_mime_types, neither
will generate a files.log entry when I am not running a PCAP from the
command line. So any kind of normal network processing and/or playing a
pcap over a listening interface via tcpreplay will cause the analyzer to
fire properly.

However, if I attach the EXTRACT analyzer, all processing goes as expected.
What nuance could I be missing here? The plugin effectively initializes
like the existing file analysis analyzers, save it’s a plugin. Is there a
hard and fast requirement that ignorer for the file analysis framework to
work properly the file has to be explicitly extracted? Most additional
analysis, I would assume not use disk resources to extract them and,
instead, observe what I need and move on.

Any insight from anyone would be greatly appreciated.

Thank you,

Aaron

On October 13, 2017 at 11:32:46 AM, Azoff, Justin S (jazoff at illinois.edu)
wrote:


> On Oct 13, 2017, at 11:01 AM, Aaron Eppert <aaron.eppert at packetsled.com>
wrote:
>
> Justin,
>
> Indeed, cutting new territory is always interesting. As for the code,
>
> https://github.com/aeppert/test_file_analyzer
>
>
> File I am using for this case:
> https://www.bro.org/static/exchange-2013/faf-exercise.pcap
>
> `bro -C -r faf-exercise.pcap` after building and installing the plugin.
>
> My suspicion is it’s either unbelievably trivial and I keep missing it
because I am the only one staring at it, or it’s a rather deep rabbit hole.
>
> Aaron

Thanks for putting that together.. now I see what you mean. Building the
plugin with ASAN confirms it is trying to access uninitialized memory:


$ /usr/local/bro/bin/bro -C -r faf-exercise.pcap
TEST::Finalize total_len = 65960
BUFFER
00 ea 09 00 50 61 00 00 80 eb 09 00 50 61 00 00
=================================================================
==93650==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x603000bf5d08 at pc 0x00010b19d39b bp 0x7fff57829e10 sp 0x7fff57829e08
READ of size 1 at 0x603000bf5d08 thread T0
#0 0x10b19d39a in print_bytes(std::__1::basic_ostream<char,
std::__1::char_traits<char> >&, char const*, unsigned char const*, unsigned
long, bool) TEST.cc:21
#1 0x10b19e43b in file_analysis::TEST::Finalize() TEST.cc:87

...



The problem is this line:

bufv->push_back(data);

That's only pushing the first char of the buffer onto the vector, not the
entire buffer.

If you print out bufv->size() you'll see that it is not what it should be.

If you apply this change it will run without crashing and I believe give
the expected output:

diff --git a/src/TEST.cc b/src/TEST.cc
index 8d78ef2..56d0a83 100644
--- a/src/TEST.cc
+++ b/src/TEST.cc
@@ -56,7 +56,7 @@ bool TEST::DeliverStream(const u_char* data, uint64 len)
}

if ( total_len < TEST_MAX_BUFFER) {
- bufv->push_back(data);
+ print_bytes(std::cout, "BUFFER", data, len);
total_len += len;
}

@@ -84,7 +84,7 @@ void TEST::Finalize()

//auto pos = std::find(bufv->begin(), bufv->end(), (unsigned char
*)"Exif");
//std::cout << "Offset = " << std::distance( bufv->begin(), pos ) <<
std::endl;
- print_bytes(std::cout, "BUFFER", (const u_char *)&bufv[0], total_len);
+ //print_bytes(std::cout, "BUFFER", (const u_char *)&bufv[0], total_len);

val_list* vl = new val_list();
vl->append(GetFile()->GetVal()->Ref());

I don't know off the top of my head the right way to extend a c++ vector by
a c buffer, but doing so should fix things.

—
Justin Azoff
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mailman.icsi.berkeley.edu/pipermail/bro-dev/attachments/20171102/68300617/attachment.html