diff --git a/CHANGES b/CHANGES index 6213fec522..e149b3f4d0 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,28 @@ +2.2-beta-73 | 2013-10-14 14:28:25 -0700 + + * Fix misc. Coverity-reported issues (leaks, potential null pointer + deref, dead code, uninitialized values, + time-of-check-time-of-use). (Jon Siwek) + + * Add check for sqlite3 command to tests that require it. (Daniel + Thayer) + +2.2-beta-68 | 2013-10-14 09:26:09 -0700 + + * Add check for curl command to active-http.test. (Daniel Thayer) + +2.2-beta-64 | 2013-10-14 09:20:04 -0700 + + * Review usage of Reporter::InternalError, addresses BIT-1045. + + Replaced some with InternalWarning or AnalyzerError, the later + being a new method which signals the analyzer to not process + further input. (Jon Siwek) + + * Add new event for TCP content file write failures: + "contents_file_write_failure". (Jon Siwek) + 2.2-beta-57 | 2013-10-11 17:23:25 -0700 * Improve Broxygen end-of-sentence detection. (Jon Siwek) diff --git a/VERSION b/VERSION index 18d1c09781..82d1470bc0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.2-beta-57 +2.2-beta-73 diff --git a/aux/binpac b/aux/binpac index 877d92eaaf..923994715b 160000 --- a/aux/binpac +++ b/aux/binpac @@ -1 +1 @@ -Subproject commit 877d92eaafb474aa8f34e5961a726644b4968303 +Subproject commit 923994715b34bf3292e402bbe00c00ff77556490 diff --git a/aux/bro-aux b/aux/bro-aux index b3f1fda060..1496e0319f 160000 --- a/aux/bro-aux +++ b/aux/bro-aux @@ -1 +1 @@ -Subproject commit b3f1fda060492ff4766c27535f49178739fbb926 +Subproject commit 1496e0319f6fa12bb39362ab0947c82e1d6c669b diff --git a/aux/broccoli b/aux/broccoli index 39ae475906..ea895c1d8d 160000 --- a/aux/broccoli +++ b/aux/broccoli @@ -1 +1 @@ -Subproject commit 39ae47590617083533640d5d03b2454d6959a8a7 +Subproject commit ea895c1d8d97f4202388dc528c4bb884558a75a1 diff --git a/aux/broctl b/aux/broctl index 81fa2d664a..2ccc76eee5 160000 --- a/aux/broctl +++ b/aux/broctl @@ -1 +1 @@ -Subproject commit 81fa2d664a7ef7306a03928484b10611fbe893b8 +Subproject commit 2ccc76eee5565b7142e10f9b625a05e9932f459f diff --git a/cmake b/cmake index c966aecf2b..d902e23fd1 160000 --- a/cmake +++ b/cmake @@ -1 +1 @@ -Subproject commit c966aecf2bc83f7bbfdd1ac716c6627dd95cb2ec +Subproject commit d902e23fd14624eb9caf0b4a0e693014bf5bd684 diff --git a/src/Conn.cc b/src/Conn.cc index e221bb20e0..0098d297e0 100644 --- a/src/Conn.cc +++ b/src/Conn.cc @@ -782,8 +782,15 @@ void Connection::Describe(ODesc* d) const break; case TRANSPORT_UNKNOWN: - reporter->InternalError("unknown transport in Connction::Describe()"); + d->Add("unknown"); + reporter->InternalWarning( + "unknown transport in Connction::Describe()"); + break; + + default: + reporter->InternalError( + "unhandled transport type in Connection::Describe"); } d->SP(); diff --git a/src/DNS_Mgr.cc b/src/DNS_Mgr.cc index c1036accdd..17409a930b 100644 --- a/src/DNS_Mgr.cc +++ b/src/DNS_Mgr.cc @@ -546,7 +546,7 @@ Val* DNS_Mgr::LookupAddr(const IPAddr& addr) return LookupAddr(addr); default: - reporter->InternalError("bad mode in DNS_Mgr::LookupHost"); + reporter->InternalError("bad mode in DNS_Mgr::LookupAddr"); return 0; } } @@ -833,7 +833,10 @@ void DNS_Mgr::CompareMappings(DNS_Mapping* prev_dm, DNS_Mapping* new_dm) ListVal* new_a = new_dm->Addrs(); if ( ! prev_a || ! new_a ) - reporter->InternalError("confused in DNS_Mgr::CompareMappings"); + { + reporter->InternalWarning("confused in DNS_Mgr::CompareMappings"); + return; + } ListVal* prev_delta = AddrListDelta(prev_a, new_a); ListVal* new_delta = AddrListDelta(new_a, prev_a); diff --git a/src/Desc.cc b/src/Desc.cc index 40b6fcc3d1..62c6130f40 100644 --- a/src/Desc.cc +++ b/src/Desc.cc @@ -66,6 +66,7 @@ void ODesc::PopIndent() { if ( --indent_level < 0 ) reporter->InternalError("ODesc::PopIndent underflow"); + NL(); } diff --git a/src/Event.cc b/src/Event.cc index d6bb0d8e58..0fb76d10bc 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -91,7 +91,7 @@ void EventMgr::QueueEvent(Event* event) void EventMgr::Dispatch() { if ( ! head ) - reporter->InternalError("EventMgr underflow"); + reporter->InternalError("EventMgr::Dispatch underflow"); Event* current = head; diff --git a/src/EventRegistry.cc b/src/EventRegistry.cc index 2da16de51d..2428198255 100644 --- a/src/EventRegistry.cc +++ b/src/EventRegistry.cc @@ -90,9 +90,14 @@ void EventRegistry::PrintDebug() void EventRegistry::SetErrorHandler(const char* name) { EventHandler* eh = Lookup(name); - if ( ! eh ) - reporter->InternalError("unknown event handler in SetErrorHandler()"); - eh->SetErrorHandler(); + if ( eh ) + { + eh->SetErrorHandler(); + return; + } + + reporter->InternalWarning( + "unknown event handler '%s' in SetErrorHandler()", name); } diff --git a/src/File.cc b/src/File.cc index 880fd254ef..bf6a7e7f51 100644 --- a/src/File.cc +++ b/src/File.cc @@ -281,6 +281,8 @@ FILE* BroFile::File() FILE* BroFile::BringIntoCache() { + char buf[256]; + if ( f ) reporter->InternalError("BroFile non-nil non-open file"); @@ -296,22 +298,30 @@ FILE* BroFile::BringIntoCache() if ( ! f ) { - reporter->Error("can't open %s", name); + strerror_r(errno, buf, sizeof(buf)); + reporter->Error("can't open %s: %s", name, buf); f = fopen("/dev/null", "w"); - if ( ! f ) - reporter->InternalError("out of file descriptors"); + if ( f ) + { + okay_to_manage = 0; + return f; + } - okay_to_manage = 0; - return f; + strerror_r(errno, buf, sizeof(buf)); + reporter->Error("can't open /dev/null: %s", buf); + return 0; } RaiseOpenEvent(); UpdateFileSize(); if ( fseek(f, position, SEEK_SET) < 0 ) - reporter->Error("reopen seek failed"); + { + strerror_r(errno, buf, sizeof(buf)); + reporter->Error("reopen seek failed: %s", buf); + } InsertAtBeginning(); @@ -387,6 +397,7 @@ void BroFile::Suspend() { if ( ! is_in_cache ) reporter->InternalError("BroFile::Suspend() called for non-cached file"); + if ( ! is_open ) reporter->InternalError("BroFile::Suspend() called for non-open file"); @@ -397,7 +408,9 @@ void BroFile::Suspend() if ( (position = ftell(f)) < 0 ) { - reporter->Error("ftell failed"); + char buf[256]; + strerror_r(errno, buf, sizeof(buf)); + reporter->Error("ftell failed: %s", buf); position = 0; } @@ -407,10 +420,13 @@ void BroFile::Suspend() void BroFile::PurgeCache() { - if ( ! tail ) - reporter->InternalError("BroFile purge of empty cache"); + if ( tail ) + { + tail->Suspend(); + return; + } - tail->Suspend(); + reporter->InternalWarning("BroFile purge of empty cache"); } void BroFile::Unlink() diff --git a/src/FlowSrc.cc b/src/FlowSrc.cc index 7a79e9063b..32aa4c4e3a 100644 --- a/src/FlowSrc.cc +++ b/src/FlowSrc.cc @@ -14,8 +14,11 @@ FlowSrc::FlowSrc() { // TODO: v9. + selectable_fd = -1; idle = false; data = 0; + pdu_len = -1; + exporter_ip = 0; current_timestamp = next_timestamp = 0.0; netflow_analyzer = new binpac::NetFlow::NetFlow_Analyzer(); } diff --git a/src/Frag.cc b/src/Frag.cc index 4b9047d072..b1efb41594 100644 --- a/src/Frag.cc +++ b/src/Frag.cc @@ -22,7 +22,7 @@ void FragTimer::Dispatch(double t, int /* is_expire */) if ( f ) f->Expire(t); else - reporter->InternalError("fragment timer dispatched w/o reassembler"); + reporter->InternalWarning("fragment timer dispatched w/o reassembler"); } FragReassembler::FragReassembler(NetSessions* arg_s, @@ -155,14 +155,35 @@ void FragReassembler::AddFragment(double t, const IP_Hdr* ip, const u_char* pkt) NewBlock(network_time, offset, len, pkt); } +void FragReassembler::Weird(const char* name) const + { + unsigned int version = ((const ip*)proto_hdr)->ip_v; + + if ( version == 4 ) + { + IP_Hdr hdr((const ip*)proto_hdr, false); + s->Weird(name, &hdr); + } + + else if ( version == 6 ) + { + IP_Hdr hdr((const ip6_hdr*)proto_hdr, false, proto_hdr_len); + s->Weird(name, &hdr); + } + + else + { + reporter->InternalWarning("Unexpected IP version in FragReassembler"); + reporter->Weird(name); + } + } + void FragReassembler::Overlap(const u_char* b1, const u_char* b2, int n) { - IP_Hdr proto_h(proto_hdr, false, proto_hdr_len); - if ( memcmp((const void*) b1, (const void*) b2, n) ) - s->Weird("fragment_inconsistency", &proto_h); + Weird("fragment_inconsistency"); else - s->Weird("fragment_overlap", &proto_h); + Weird("fragment_overlap"); } void FragReassembler::BlockInserted(DataBlock* /* start_block */) @@ -188,9 +209,7 @@ void FragReassembler::BlockInserted(DataBlock* /* start_block */) // beyond it, which is not contiguous. This // can happen for benign reasons when we're // intermingling parts of two fragmented packets. - - IP_Hdr proto_h(proto_hdr, false, proto_hdr_len); - s->Weird("fragment_size_inconsistency", &proto_h); + Weird("fragment_size_inconsistency"); // We decide to analyze the contiguous portion now. // Extend the fragment up through the end of what @@ -203,8 +222,7 @@ void FragReassembler::BlockInserted(DataBlock* /* start_block */) else if ( last_block->upper > frag_size ) { - IP_Hdr proto_h(proto_hdr, false, proto_hdr_len); - s->Weird("fragment_size_inconsistency", &proto_h); + Weird("fragment_size_inconsistency"); frag_size = last_block->upper; } @@ -238,36 +256,45 @@ void FragReassembler::BlockInserted(DataBlock* /* start_block */) break; if ( b->upper > n ) - reporter->InternalError("bad fragment reassembly"); + { + reporter->InternalWarning("bad fragment reassembly"); + DeleteTimer(); + Expire(network_time); + return; + } memcpy((void*) &pkt[b->seq], (const void*) b->block, b->upper - b->seq); } delete reassembled_pkt; + reassembled_pkt = 0; - if ( ((const struct ip*)pkt_start)->ip_v == 4 ) + unsigned int version = ((const struct ip*)pkt_start)->ip_v; + + if ( version == 4 ) { struct ip* reassem4 = (struct ip*) pkt_start; reassem4->ip_len = htons(frag_size + proto_hdr_len); reassembled_pkt = new IP_Hdr(reassem4, true); + DeleteTimer(); } - else if ( ((const struct ip*)pkt_start)->ip_v == 6 ) + else if ( version == 6 ) { struct ip6_hdr* reassem6 = (struct ip6_hdr*) pkt_start; reassem6->ip6_plen = htons(frag_size + proto_hdr_len - 40); const IPv6_Hdr_Chain* chain = new IPv6_Hdr_Chain(reassem6, next_proto, n); reassembled_pkt = new IP_Hdr(reassem6, true, n, chain); + DeleteTimer(); } else { - reporter->InternalError("bad IP version in fragment reassembly"); + reporter->InternalWarning("bad IP version in fragment reassembly: %d", + version); + delete [] pkt_start; } - - - DeleteTimer(); } void FragReassembler::Expire(double t) diff --git a/src/Frag.h b/src/Frag.h index 86cf3a9dd4..0d2fbed5b3 100644 --- a/src/Frag.h +++ b/src/Frag.h @@ -35,6 +35,7 @@ public: protected: void BlockInserted(DataBlock* start_block); void Overlap(const u_char* b1, const u_char* b2, int n); + void Weird(const char* name) const; u_char* proto_hdr; IP_Hdr* reassembled_pkt; diff --git a/src/ID.cc b/src/ID.cc index a6e592146b..7a3940e8c0 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -440,7 +440,6 @@ ID* ID::Unserialize(UnserialInfo* info) default: reporter->InternalError("unknown type for UnserialInfo::id_policy"); - } } @@ -543,7 +542,7 @@ bool ID::DoUnserialize(UnserialInfo* info) } if ( installed_tmp && ! global_scope()->Remove(name) ) - reporter->InternalError("tmp id missing"); + reporter->InternalWarning("missing tmp ID in %s unserialization", name); return true; } diff --git a/src/IP.cc b/src/IP.cc index 78311cc2d2..46702f2546 100644 --- a/src/IP.cc +++ b/src/IP.cc @@ -433,7 +433,10 @@ void IPv6_Hdr_Chain::Init(const struct ip6_hdr* ip6, int total_len, const u_char* hdrs = (const u_char*) ip6; if ( total_len < (int)sizeof(struct ip6_hdr) ) - reporter->InternalError("IPv6_HdrChain::Init with truncated IP header"); + { + reporter->InternalWarning("truncated IP header in IPv6_HdrChain::Init"); + return; + } do { @@ -623,9 +626,11 @@ VectorVal* IPv6_Hdr_Chain::BuildVal() const break; #endif default: - reporter->InternalError("IPv6_Hdr_Chain bad header %d", type); - break; + reporter->InternalWarning("IPv6_Hdr_Chain bad header %d", type); + Unref(ext_hdr); + continue; } + rval->Assign(rval->Size(), ext_hdr); } diff --git a/src/IP.h b/src/IP.h index c3a74b4a01..fb936df1bc 100644 --- a/src/IP.h +++ b/src/IP.h @@ -176,7 +176,15 @@ public: * Returns whether the header chain indicates a fragmented packet. */ bool IsFragment() const - { return chain[chain.size()-1]->Type() == IPPROTO_FRAGMENT; } + { + if ( chain.empty() ) + { + reporter->InternalWarning("empty IPv6 header chain"); + return false; + } + + return chain[chain.size()-1]->Type() == IPPROTO_FRAGMENT; + } /** * Returns pointer to fragment header structure if the chain contains one. @@ -216,9 +224,14 @@ public: #ifdef ENABLE_MOBILE_IPV6 if ( homeAddr ) return IPAddr(*homeAddr); - else #endif - return IPAddr(((const struct ip6_hdr*)(chain[0]->Data()))->ip6_src); + if ( chain.empty() ) + { + reporter->InternalWarning("empty IPv6 header chain"); + return IPAddr(); + } + + return IPAddr(((const struct ip6_hdr*)(chain[0]->Data()))->ip6_src); } /** @@ -230,8 +243,14 @@ public: { if ( finalDst ) return IPAddr(*finalDst); - else - return IPAddr(((const struct ip6_hdr*)(chain[0]->Data()))->ip6_dst); + + if ( chain.empty() ) + { + reporter->InternalWarning("empty IPv6 header chain"); + return IPAddr(); + } + + return IPAddr(((const struct ip6_hdr*)(chain[0]->Data()))->ip6_dst); } /** @@ -305,32 +324,6 @@ protected: */ class IP_Hdr { public: - /** - * Attempts to construct the header from some blob of data based on IP - * version number. Caller must have already checked that the header - * is not truncated. - * @param p pointer to memory containing an IPv4 or IPv6 packet. - * @param arg_del whether to take ownership of \a p pointer's memory. - * @param len the length of data, in bytes, pointed to by \a p. - */ - IP_Hdr(const u_char* p, bool arg_del, int len) - : ip4(0), ip6(0), del(arg_del), ip6_hdrs(0) - { - if ( ((const struct ip*)p)->ip_v == 4 ) - ip4 = (const struct ip*)p; - else if ( ((const struct ip*)p)->ip_v == 6 ) - { - ip6 = (const struct ip6_hdr*)p; - ip6_hdrs = new IPv6_Hdr_Chain(ip6, len); - } - else - { - if ( arg_del ) - delete [] p; - reporter->InternalError("bad IP version in IP_Hdr ctor"); - } - } - /** * Construct the header wrapper from an IPv4 packet. Caller must have * already checked that the header is not truncated. @@ -365,15 +358,12 @@ public: */ ~IP_Hdr() { - if ( ip6 ) - delete ip6_hdrs; + delete ip6_hdrs; if ( del ) { - if ( ip4 ) - delete [] (struct ip*) ip4; - else - delete [] (struct ip6_hdr*) ip6; + delete [] (struct ip*) ip4; + delete [] (struct ip6_hdr*) ip6; } } @@ -472,8 +462,16 @@ public: * For IPv6 header chains, returns the type of the last header in the chain. */ uint8 LastHeader() const - { return ip4 ? IPPROTO_RAW : - ((*ip6_hdrs)[ip6_hdrs->Size()-1])->Type(); } + { + if ( ip4 ) + return IPPROTO_RAW; + + size_t i = ip6_hdrs->Size(); + if ( i > 0 ) + return (*ip6_hdrs)[i-1]->Type(); + + return IPPROTO_NONE; + } /** * Returns the protocol type of the IP packet's payload, usually an @@ -481,8 +479,16 @@ public: * header's Next Header value. */ unsigned char NextProto() const - { return ip4 ? ip4->ip_p : - ((*ip6_hdrs)[ip6_hdrs->Size()-1])->NextHdr(); } + { + if ( ip4 ) + return ip4->ip_p; + + size_t i = ip6_hdrs->Size(); + if ( i > 0 ) + return (*ip6_hdrs)[i-1]->NextHdr(); + + return IPPROTO_NONE; + } /** * Returns the IPv4 Time to Live or IPv6 Hop Limit field. diff --git a/src/OpaqueVal.cc b/src/OpaqueVal.cc index b7ccd770ce..ea0c73d297 100644 --- a/src/OpaqueVal.cc +++ b/src/OpaqueVal.cc @@ -36,7 +36,7 @@ bool HashVal::Feed(const void* data, size_t size) if ( valid ) return DoFeed(data, size); - reporter->InternalError("invalid opaque hash value"); + Error("attempt to update an invalid opaque hash value"); return false; } diff --git a/src/PacketSort.cc b/src/PacketSort.cc index a7e2b04572..429d8e2720 100644 --- a/src/PacketSort.cc +++ b/src/PacketSort.cc @@ -17,6 +17,9 @@ PacketSortElement::PacketSortElement(PktSrc* arg_src, is_tcp = 0; ip_hdr = 0; + tcp_flags = 0; + endp = 0; + payload_length = 0; key = 0; // Now check if it is a "parsable" TCP packet. diff --git a/src/PersistenceSerializer.h b/src/PersistenceSerializer.h index dcd712bf84..7274e60569 100644 --- a/src/PersistenceSerializer.h +++ b/src/PersistenceSerializer.h @@ -113,7 +113,8 @@ protected: conns = 0; conn_cookie = 0; peer = SOURCE_LOCAL; - }; + filename = 0; + } Type type; SerialInfo info; diff --git a/src/PktSrc.cc b/src/PktSrc.cc index f318405920..9d6bce6fe9 100644 --- a/src/PktSrc.cc +++ b/src/PktSrc.cc @@ -648,6 +648,7 @@ PktDumper::PktDumper(const char* arg_filename, bool arg_append) is_error = false; append = arg_append; dumper = 0; + open_time = 0.0; // We need a pcap_t with a reasonable link-layer type. We try to get it // from the packet sources. If not available, we fall back to Ethernet. diff --git a/src/PolicyFile.cc b/src/PolicyFile.cc index 7ed0b02d75..5d0082c6a9 100644 --- a/src/PolicyFile.cc +++ b/src/PolicyFile.cc @@ -86,6 +86,7 @@ bool LoadPolicyFileText(const char* policy_filename) char buf[256]; strerror_r(errno, buf, sizeof(buf)); reporter->Error("fstat failed on %s: %s", policy_filename, buf); + fclose(f); return false; } diff --git a/src/PrefixTable.cc b/src/PrefixTable.cc index 58a488a9f1..d31203c9d3 100644 --- a/src/PrefixTable.cc +++ b/src/PrefixTable.cc @@ -20,7 +20,10 @@ void* PrefixTable::Insert(const IPAddr& addr, int width, void* data) Deref_Prefix(prefix); if ( ! node ) - reporter->InternalError("Cannot create node in patricia tree"); + { + reporter->InternalWarning("Cannot create node in patricia tree"); + return 0; + } void* old = node->data; @@ -49,7 +52,7 @@ void* PrefixTable::Insert(const Val* value, void* data) break; default: - reporter->InternalError("Wrong index type for PrefixTable"); + reporter->InternalWarning("Wrong index type for PrefixTable"); return 0; } } @@ -83,8 +86,8 @@ void* PrefixTable::Lookup(const Val* value, bool exact) const break; default: - reporter->InternalError("Wrong index type %d for PrefixTable", - value->Type()->Tag()); + reporter->InternalWarning("Wrong index type %d for PrefixTable", + value->Type()->Tag()); return 0; } } @@ -122,7 +125,7 @@ void* PrefixTable::Remove(const Val* value) break; default: - reporter->InternalError("Wrong index type for PrefixTable"); + reporter->InternalWarning("Wrong index type for PrefixTable"); return 0; } } diff --git a/src/Reassem.cc b/src/Reassem.cc index c3c19ff0e6..19beaa0a16 100644 --- a/src/Reassem.cc +++ b/src/Reassem.cc @@ -19,10 +19,7 @@ DataBlock::DataBlock(const u_char* data, int size, int arg_seq, { seq = arg_seq; upper = seq + size; - block = new u_char[size]; - if ( ! block ) - reporter->InternalError("out of memory"); memcpy((void*) block, (const void*) data, size); diff --git a/src/RemoteSerializer.cc b/src/RemoteSerializer.cc index 43a963ab21..c8cf03667b 100644 --- a/src/RemoteSerializer.cc +++ b/src/RemoteSerializer.cc @@ -3321,6 +3321,9 @@ SocketComm::SocketComm() id_counter = 10000; parent_peer = 0; parent_msgstate = TYPE; + parent_id = RemoteSerializer::PEER_NONE; + parent_msgtype = 0; + parent_args = 0; shutting_conns_down = false; terminating = false; killing = false; @@ -3584,8 +3587,6 @@ bool SocketComm::ProcessParentMessage() InternalError(fmt("unknown msg type %d", parent_msgtype)); return true; } - - InternalError("cannot be reached"); } case ARGS: @@ -3611,7 +3612,7 @@ bool SocketComm::ProcessParentMessage() InternalError("unknown msgstate"); } - InternalError("cannot be reached"); + // Cannot be reached. return false; } diff --git a/src/Reporter.cc b/src/Reporter.cc index 6bc2577c72..9002633b10 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -137,11 +137,27 @@ void Reporter::InternalError(const char* fmt, ...) abort(); } +void Reporter::AnalyzerError(analyzer::Analyzer* a, const char* fmt, + ...) + { + if ( a ) + a->SetSkip(true); + + va_list ap; + va_start(ap, fmt); + // Always log to stderr. + // TODO: would be nice to also log a call stack. + DoLog("analyzer error", reporter_error, stderr, 0, 0, true, true, 0, fmt, + ap); + va_end(ap); + } + void Reporter::InternalWarning(const char* fmt, ...) { va_list ap; va_start(ap, fmt); FILE* out = warnings_to_stderr ? stderr : 0; + // TODO: would be nice to also log a call stack. DoLog("internal warning", reporter_warning, out, 0, 0, true, true, 0, fmt, ap); va_end(ap); diff --git a/src/Reporter.h b/src/Reporter.h index 6a205cfd6d..e477ad8934 100644 --- a/src/Reporter.h +++ b/src/Reporter.h @@ -12,6 +12,7 @@ #include "EventHandler.h" #include "IPAddr.h" +namespace analyzer { class Analyzer; } class Connection; class Location; class Reporter; @@ -91,6 +92,10 @@ public: // dump after the message has been reported. void InternalError(const char* fmt, ...) FMT_ATTR; + // Report an analyzer error. That analyzer will be set to not process + // any further input, but Bro otherwise continues normally. + void AnalyzerError(analyzer::Analyzer* a, const char* fmt, ...); + // Toggle whether non-fatal messages should be reported through the // scripting layer rather on standard output. Fatal errors are always // reported via stderr. diff --git a/src/RuleMatcher.cc b/src/RuleMatcher.cc index ed33db6792..49c6460ea7 100644 --- a/src/RuleMatcher.cc +++ b/src/RuleMatcher.cc @@ -521,7 +521,7 @@ static inline bool compare(const maskedvalue_list& mvals, uint32 v, break; default: - reporter->InternalError("unknown comparison type"); + reporter->InternalError("unknown RuleHdrTest comparison type"); break; } return false; @@ -556,7 +556,7 @@ static inline bool compare(const vector& prefixes, const IPAddr& a, break; default: - reporter->InternalError("unknown comparison type"); + reporter->InternalError("unknown RuleHdrTest comparison type"); break; } return false; @@ -661,7 +661,7 @@ RuleEndpointState* RuleMatcher::InitEndpoint(analyzer::Analyzer* analyzer, break; default: - reporter->InternalError("unknown protocol"); + reporter->InternalError("unknown RuleHdrTest protocol type"); break; } diff --git a/src/Serializer.cc b/src/Serializer.cc index 3f0a66a408..156ad67f2e 100644 --- a/src/Serializer.cc +++ b/src/Serializer.cc @@ -419,7 +419,7 @@ bool Serializer::UnserializeCall(UnserialInfo* info) break; default: - reporter->InternalError("invalid function flavor"); + reporter->InternalError("unserialized call for invalid function flavor"); break; } diff --git a/src/Sessions.cc b/src/Sessions.cc index b844dc614e..acc306d277 100644 --- a/src/Sessions.cc +++ b/src/Sessions.cc @@ -781,7 +781,10 @@ int NetSessions::ParseIPPacket(int caplen, const u_char* const pkt, int proto, } else - reporter->InternalError("Bad IP protocol version in DoNextInnerPacket"); + { + reporter->InternalWarning("Bad IP protocol version in ParseIPPacket"); + return -1; + } if ( (uint32)caplen != inner->TotalLen() ) return (uint32)caplen < inner->TotalLen() ? -1 : 1; @@ -993,21 +996,21 @@ void NetSessions::Remove(Connection* c) switch ( c->ConnTransport() ) { case TRANSPORT_TCP: if ( ! tcp_conns.RemoveEntry(k) ) - reporter->InternalError("connection missing"); + reporter->InternalWarning("connection missing"); break; case TRANSPORT_UDP: if ( ! udp_conns.RemoveEntry(k) ) - reporter->InternalError("connection missing"); + reporter->InternalWarning("connection missing"); break; case TRANSPORT_ICMP: if ( ! icmp_conns.RemoveEntry(k) ) - reporter->InternalError("connection missing"); + reporter->InternalWarning("connection missing"); break; case TRANSPORT_UNKNOWN: - reporter->InternalError("unknown transport when removing connection"); + reporter->InternalWarning("unknown transport when removing connection"); break; } @@ -1018,13 +1021,18 @@ void NetSessions::Remove(Connection* c) void NetSessions::Remove(FragReassembler* f) { - if ( ! f ) return; - HashKey* k = f->Key(); - if ( ! k ) - reporter->InternalError("fragment block not in dictionary"); + if ( ! f ) + return; - if ( ! fragments.RemoveEntry(k) ) - reporter->InternalError("fragment block missing"); + HashKey* k = f->Key(); + + if ( k ) + { + if ( ! fragments.RemoveEntry(k) ) + reporter->InternalWarning("fragment reassembler not in dict"); + } + else + reporter->InternalWarning("missing fragment reassembler hash key"); Unref(f); } @@ -1055,7 +1063,9 @@ void NetSessions::Insert(Connection* c) break; default: - reporter->InternalError("unknown connection type"); + reporter->InternalWarning("unknown connection type"); + Unref(c); + return; } if ( old && old != c ) @@ -1147,8 +1157,8 @@ Connection* NetSessions::NewConn(HashKey* k, double t, const ConnID* id, tproto = TRANSPORT_ICMP; break; default: - reporter->InternalError("unknown transport protocol"); - break; + reporter->InternalWarning("unknown transport protocol"); + return 0; }; if ( tproto == TRANSPORT_TCP ) @@ -1181,7 +1191,13 @@ Connection* NetSessions::NewConn(HashKey* k, double t, const ConnID* id, Connection* conn = new Connection(this, k, t, id, flow_label, encapsulation); conn->SetTransport(tproto); - analyzer_mgr->BuildInitialAnalyzerTree(conn); + + if ( ! analyzer_mgr->BuildInitialAnalyzerTree(conn) ) + { + conn->Done(); + Unref(conn); + return 0; + } bool external = conn->IsExternal(); diff --git a/src/Stmt.cc b/src/Stmt.cc index d879c598d2..3571cad197 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -799,8 +799,9 @@ int SwitchStmt::FindCaseLabelMatch(const Val* v) const if ( ! hk ) { reporter->PushLocation(e->GetLocationInfo()); - reporter->InternalError("switch expression type mismatch (%s/%s)", + reporter->Error("switch expression type mismatch (%s/%s)", type_name(v->Type()->Tag()), type_name(e->Type()->Tag())); + return -1; } int* label_idx = case_label_map.Lookup(hk); diff --git a/src/Val.cc b/src/Val.cc index d791ff195d..450f3c1653 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -238,7 +238,6 @@ bool Val::DoSerialize(SerialInfo* info) const return false; } - reporter->InternalError("should not be reached"); return false; } @@ -316,7 +315,6 @@ bool Val::DoUnserialize(UnserialInfo* info) return false; } - reporter->InternalError("should not be reached"); return false; } @@ -519,8 +517,9 @@ void Val::ValDescribe(ODesc* d) const break; default: - // Don't call Internal(), that'll loop! - reporter->InternalError("Val description unavailable"); + reporter->InternalWarning("Val description unavailable"); + d->Add(""); + break; } } @@ -1893,7 +1892,7 @@ Val* TableVal::Delete(const Val* index) Val* va = v ? (v->Value() ? v->Value() : this->Ref()) : 0; if ( subnets && ! subnets->Remove(index) ) - reporter->InternalError("index not in prefix table"); + reporter->InternalWarning("index not in prefix table"); if ( LoggingAccess() ) { @@ -1935,7 +1934,7 @@ Val* TableVal::Delete(const HashKey* k) { Val* index = table_hash->RecoverVals(k); if ( ! subnets->Remove(index) ) - reporter->InternalError("index not in prefix table"); + reporter->InternalWarning("index not in prefix table"); Unref(index); } @@ -2195,7 +2194,7 @@ void TableVal::DoExpire(double t) { Val* index = RecoverIndex(k); if ( ! subnets->Remove(index) ) - reporter->InternalError("index not in prefix table"); + reporter->InternalWarning("index not in prefix table"); Unref(index); } @@ -3300,7 +3299,7 @@ int same_atomic_val(const Val* v1, const Val* v2) return v1->AsSubNet() == v2->AsSubNet(); default: - reporter->InternalError("same_atomic_val called for non-atomic value"); + reporter->InternalWarning("same_atomic_val called for non-atomic value"); return 0; } diff --git a/src/analyzer/Analyzer.h b/src/analyzer/Analyzer.h index b709e3dda0..f7ca07ca51 100644 --- a/src/analyzer/Analyzer.h +++ b/src/analyzer/Analyzer.h @@ -303,7 +303,7 @@ public: * Signals the analyzer to skip all further input processsing. The \a * Next*() methods check this flag and discard the input if its set. * - * @param do_skipe If true, further processing will be skipped. + * @param do_skip If true, further processing will be skipped. */ void SetSkip(bool do_skip) { skip = do_skip; } diff --git a/src/analyzer/Manager.cc b/src/analyzer/Manager.cc index 2359e4ec98..6268fde177 100644 --- a/src/analyzer/Manager.cc +++ b/src/analyzer/Manager.cc @@ -250,6 +250,9 @@ bool Manager::RegisterAnalyzerForPort(Tag tag, TransportProto proto, uint32 port { tag_set* l = LookupPort(proto, port, true); + if ( ! l ) + return false; + #ifdef DEBUG const char* name = GetComponentName(tag); DBG_LOG(DBG_ANALYZER, "Registering analyzer %s for port %" PRIu32 "/%d", name, port, proto); @@ -263,6 +266,9 @@ bool Manager::UnregisterAnalyzerForPort(Tag tag, TransportProto proto, uint32 po { tag_set* l = LookupPort(proto, port, true); + if ( ! l ) + return true; // still a "successful" unregistration + #ifdef DEBUG const char* name = GetComponentName(tag); DBG_LOG(DBG_ANALYZER, "Unregistering analyzer %s for port %" PRIu32 "/%d", name, port, proto); @@ -277,18 +283,27 @@ Analyzer* Manager::InstantiateAnalyzer(Tag tag, Connection* conn) Component* c = Lookup(tag); if ( ! c ) - reporter->InternalError("request to instantiate unknown analyzer"); + { + reporter->InternalWarning("request to instantiate unknown analyzer"); + return 0; + } if ( ! c->Enabled() ) return 0; if ( ! c->Factory() ) - reporter->InternalError("analyzer %s cannot be instantiated dynamically", GetComponentName(tag)); + { + reporter->InternalWarning("analyzer %s cannot be instantiated dynamically", GetComponentName(tag)); + return 0; + } Analyzer* a = c->Factory()(conn); if ( ! a ) - reporter->InternalError("analyzer instantiation failed"); + { + reporter->InternalWarning("analyzer instantiation failed"); + return 0; + } a->SetAnalyzerTag(tag); @@ -315,7 +330,8 @@ Manager::tag_set* Manager::LookupPort(TransportProto proto, uint32 port, bool ad break; default: - reporter->InternalError("unsupport transport protocol in analyzer::Manager::LookupPort"); + reporter->InternalWarning("unsupported transport protocol in analyzer::Manager::LookupPort"); + return 0; } analyzer_map_by_port::const_iterator i = m->find(port); @@ -338,7 +354,6 @@ Manager::tag_set* Manager::LookupPort(PortVal* val, bool add_if_not_found) bool Manager::BuildInitialAnalyzerTree(Connection* conn) { - Analyzer* analyzer = 0; tcp::TCP_Analyzer* tcp = 0; udp::UDP_Analyzer* udp = 0; icmp::ICMP_Analyzer* icmp = 0; @@ -374,14 +389,9 @@ bool Manager::BuildInitialAnalyzerTree(Connection* conn) } default: - reporter->InternalError("unknown protocol"); - } - - if ( ! root ) - { - DBG_ANALYZER(conn, "cannot build analyzer tree"); + reporter->InternalWarning("unknown protocol can't build analyzer tree"); return false; - } + } // Any scheduled analyzer? for ( tag_set::iterator i = expected.begin(); i != expected.end(); i++ ) diff --git a/src/analyzer/Manager.h b/src/analyzer/Manager.h index d151709eda..77b40a1d68 100644 --- a/src/analyzer/Manager.h +++ b/src/analyzer/Manager.h @@ -214,7 +214,8 @@ public: * * @return The new analyzer instance. Note that the analyzer will not * have been added to the connection's analyzer tree yet. Returns - * null if tag is invalid or the requested analyzer is disabled. + * null if tag is invalid, the requested analyzer is disabled, or the + * analyzer can't be instantiated. */ Analyzer* InstantiateAnalyzer(Tag tag, Connection* c); diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc index 4bb2385c27..ffdcad226f 100644 --- a/src/analyzer/protocol/http/HTTP.cc +++ b/src/analyzer/protocol/http/HTTP.cc @@ -698,7 +698,11 @@ void HTTP_Message::SubmitData(int len, const char* buf) { if ( buf != (const char*) data_buffer->Bytes() + buffer_offset || buffer_offset + len > buffer_size ) - reporter->InternalError("buffer misalignment"); + { + reporter->AnalyzerError(MyHTTP_Analyzer(), + "HTTP message buffer misalignment"); + return; + } buffer_offset += len; if ( buffer_offset >= buffer_size ) @@ -743,7 +747,9 @@ void HTTP_Message::SubmitEvent(int event_type, const char* detail) break; default: - reporter->InternalError("unrecognized HTTP message event"); + reporter->AnalyzerError(MyHTTP_Analyzer(), + "unrecognized HTTP message event"); + return; } MyHTTP_Analyzer()->HTTP_Event(category, detail); @@ -962,7 +968,13 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig) switch ( request_state ) { case EXPECT_REQUEST_LINE: - if ( HTTP_RequestLine(line, end_of_line) ) + { + int res = HTTP_RequestLine(line, end_of_line); + + if ( res < 0 ) + return; + + else if ( res > 0 ) { ++num_requests; @@ -1001,6 +1013,7 @@ void HTTP_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig) } } } + } break; case EXPECT_REQUEST_MESSAGE: @@ -1225,7 +1238,10 @@ int HTTP_Analyzer::HTTP_RequestLine(const char* line, const char* end_of_line) request_method = new StringVal(end_of_method - line, line); if ( ! ParseRequest(rest, end_of_line) ) - reporter->InternalError("HTTP ParseRequest failed"); + { + reporter->AnalyzerError(this, "HTTP ParseRequest failed"); + return -1; + } Conn()->Match(Rule::HTTP_REQUEST, (const u_char*) unescaped_URI->AsString()->Bytes(), diff --git a/src/analyzer/protocol/icmp/ICMP.cc b/src/analyzer/protocol/icmp/ICMP.cc index 17b5b0fcc1..510d3b3a80 100644 --- a/src/analyzer/protocol/icmp/ICMP.cc +++ b/src/analyzer/protocol/icmp/ICMP.cc @@ -60,9 +60,9 @@ void ICMP_Analyzer::DeliverPacket(int len, const u_char* data, break; default: - reporter->InternalError("unexpected IP proto in ICMP analyzer: %d", - ip->NextProto()); - break; + reporter->AnalyzerError(this, + "unexpected IP proto in ICMP analyzer: %d", ip->NextProto()); + return; } if ( chksum != 0xffff ) @@ -99,7 +99,11 @@ void ICMP_Analyzer::DeliverPacket(int len, const u_char* data, else if ( ip->NextProto() == IPPROTO_ICMPV6 ) NextICMP6(current_timestamp, icmpp, len, caplen, data, ip); else - reporter->InternalError("unexpected next protocol in ICMP::DeliverPacket()"); + { + reporter->AnalyzerError(this, + "expected ICMP as IP packet's protocol, got %d", ip->NextProto()); + return; + } if ( caplen >= len ) diff --git a/src/analyzer/protocol/login/Login.cc b/src/analyzer/protocol/login/Login.cc index de2445d81f..8dcb7dba55 100644 --- a/src/analyzer/protocol/login/Login.cc +++ b/src/analyzer/protocol/login/Login.cc @@ -117,7 +117,11 @@ void Login_Analyzer::NewLine(bool orig, char* line) } if ( state != LOGIN_STATE_CONFUSED ) - reporter->InternalError("bad state in Login_Analyzer::NewLine"); + { + reporter->AnalyzerError(this, + "bad state in Login_Analyzer::NewLine"); + return; + } // When we're in "confused", we feed each user input line to // login_confused_text, but also scan the text in the @@ -550,6 +554,9 @@ int Login_Analyzer::IsTimeout(const char* line) const int Login_Analyzer::IsEmpty(const char* line) const { + if ( ! line ) + return true; + while ( *line && isspace(*line) ) ++line; @@ -571,10 +578,14 @@ void Login_Analyzer::AddUserText(const char* line) } } -char* Login_Analyzer::PeekUserText() const +char* Login_Analyzer::PeekUserText() { if ( num_user_text <= 0 ) - reporter->InternalError("underflow in Login_Analyzer::PeekUserText()"); + { + reporter->AnalyzerError(this, + "underflow in Login_Analyzer::PeekUserText()"); + return 0; + } return user_text[user_text_first]; } @@ -583,6 +594,9 @@ char* Login_Analyzer::PopUserText() { char* s = PeekUserText(); + if ( ! s ) + return 0; + if ( ++user_text_first == MAX_USER_TEXT ) user_text_first = 0; @@ -594,8 +608,11 @@ char* Login_Analyzer::PopUserText() Val* Login_Analyzer::PopUserTextVal() { char* s = PopUserText(); - BroString* bs = new BroString(1, byte_vec(s), strlen(s)); - return new StringVal(bs); + + if ( s ) + return new StringVal(new BroString(1, byte_vec(s), strlen(s))); + else + return new StringVal(""); } int Login_Analyzer::MatchesTypeahead(const char* line) const diff --git a/src/analyzer/protocol/login/Login.h b/src/analyzer/protocol/login/Login.h index 2178bdff1a..25d1b24005 100644 --- a/src/analyzer/protocol/login/Login.h +++ b/src/analyzer/protocol/login/Login.h @@ -55,8 +55,8 @@ protected: int IsEmpty(const char* line) const; void AddUserText(const char* line); // complains on overflow - char* PeekUserText() const; // internal error on underflow - char* PopUserText(); // internal error on underflow + char* PeekUserText(); // internal warning on underflow + char* PopUserText(); // internal warning on underflow Val* PopUserTextVal(); int MatchesTypeahead(const char* line) const; diff --git a/src/analyzer/protocol/login/NVT.cc b/src/analyzer/protocol/login/NVT.cc index e1a3ae05ec..462cd42177 100644 --- a/src/analyzer/protocol/login/NVT.cc +++ b/src/analyzer/protocol/login/NVT.cc @@ -39,8 +39,13 @@ TelnetOption::TelnetOption(NVT_Analyzer* arg_endp, unsigned int arg_code) void TelnetOption::RecvOption(unsigned int type) { TelnetOption* peer = endp->FindPeerOption(code); + if ( ! peer ) - reporter->InternalError("option peer missing in TelnetOption::RecvOption"); + { + reporter->AnalyzerError(endp, + "option peer missing in TelnetOption::RecvOption"); + return; + } // WILL/WONT/DO/DONT are messages we've *received* from our peer. switch ( type ) { @@ -85,7 +90,9 @@ void TelnetOption::RecvOption(unsigned int type) break; default: - reporter->InternalError("bad option type in TelnetOption::RecvOption"); + reporter->AnalyzerError(endp, + "bad option type in TelnetOption::RecvOption"); + return; } } @@ -165,7 +172,11 @@ void TelnetEncryptOption::RecvSubOption(u_char* data, int len) (TelnetEncryptOption*) endp->FindPeerOption(code); if ( ! peer ) - reporter->InternalError("option peer missing in TelnetEncryptOption::RecvSubOption"); + { + reporter->AnalyzerError(endp, + "option peer missing in TelnetEncryptOption::RecvSubOption"); + return; + } if ( peer->DidRequest() || peer->DoingEncryption() || peer->Endpoint()->AuthenticationHasBeenAccepted() ) @@ -201,7 +212,11 @@ void TelnetAuthenticateOption::RecvSubOption(u_char* data, int len) (TelnetAuthenticateOption*) endp->FindPeerOption(code); if ( ! peer ) - reporter->InternalError("option peer missing in TelnetAuthenticateOption::RecvSubOption"); + { + reporter->AnalyzerError(endp, + "option peer missing in TelnetAuthenticateOption::RecvSubOption"); + return; + } if ( ! peer->DidRequestAuthentication() ) InconsistentOption(0); diff --git a/src/analyzer/protocol/login/RSH.cc b/src/analyzer/protocol/login/RSH.cc index 8aebb89116..f768f4bdc2 100644 --- a/src/analyzer/protocol/login/RSH.cc +++ b/src/analyzer/protocol/login/RSH.cc @@ -131,7 +131,8 @@ void Contents_Rsh_Analyzer::DoDeliver(int len, const u_char* data) break; default: - reporter->InternalError("bad state in Contents_Rsh_Analyzer::DoDeliver"); + reporter->AnalyzerError(this, + "bad state in Contents_Rsh_Analyzer::DoDeliver"); break; } } @@ -186,7 +187,10 @@ void Rsh_Analyzer::DeliverStream(int len, const u_char* data, bool orig) void Rsh_Analyzer::ClientUserName(const char* s) { if ( client_name ) - reporter->InternalError("multiple rsh client names"); + { + reporter->AnalyzerError(this, "multiple rsh client names"); + return; + } client_name = new StringVal(s); } @@ -194,7 +198,11 @@ void Rsh_Analyzer::ClientUserName(const char* s) void Rsh_Analyzer::ServerUserName(const char* s) { if ( username ) - reporter->InternalError("multiple rsh initial client names"); + { + reporter->AnalyzerError(this, + "multiple rsh initial client names"); + return; + } username = new StringVal(s); } diff --git a/src/analyzer/protocol/login/Rlogin.cc b/src/analyzer/protocol/login/Rlogin.cc index 9a31a47aa1..d90c9be123 100644 --- a/src/analyzer/protocol/login/Rlogin.cc +++ b/src/analyzer/protocol/login/Rlogin.cc @@ -193,7 +193,8 @@ void Contents_Rlogin_Analyzer::DoDeliver(int len, const u_char* data) break; default: - reporter->InternalError("bad state in Contents_Rlogin_Analyzer::DoDeliver"); + reporter->AnalyzerError(this, + "bad state in Contents_Rlogin_Analyzer::DoDeliver"); break; } } @@ -224,7 +225,10 @@ Rlogin_Analyzer::Rlogin_Analyzer(Connection* conn) void Rlogin_Analyzer::ClientUserName(const char* s) { if ( client_name ) - reporter->InternalError("multiple rlogin client names"); + { + reporter->AnalyzerError(this, "multiple rlogin client names"); + return; + } client_name = new StringVal(s); } diff --git a/src/analyzer/protocol/mime/MIME.cc b/src/analyzer/protocol/mime/MIME.cc index c66a0b9be7..f4e7d3981f 100644 --- a/src/analyzer/protocol/mime/MIME.cc +++ b/src/analyzer/protocol/mime/MIME.cc @@ -557,7 +557,8 @@ void MIME_Entity::init() MIME_Entity::~MIME_Entity() { if ( ! end_of_data ) - reporter->InternalError("EndOfData must be called before delete a MIME_Entity"); + reporter->AnalyzerError(message ? message->GetAnalyzer() : 0, + "missing MIME_Entity::EndOfData() before ~MIME_Entity"); delete current_header_line; Unref(content_type_str); @@ -1088,8 +1089,6 @@ void MIME_Entity::DecodeBase64(int len, const char* data) rlen = 128; char* prbuf = rbuf; int decoded = base64_decoder->Decode(len, data, &rlen, &prbuf); - if ( prbuf != rbuf ) - reporter->InternalError("buffer pointer modified in base64 decoding"); DataOctets(rlen, rbuf); len -= decoded; data += decoded; } @@ -1098,7 +1097,10 @@ void MIME_Entity::DecodeBase64(int len, const char* data) void MIME_Entity::StartDecodeBase64() { if ( base64_decoder ) - reporter->InternalError("previous Base64 decoder not released!"); + { + reporter->InternalWarning("previous MIME Base64 decoder not released"); + delete base64_decoder; + } base64_decoder = new Base64Converter(message->GetAnalyzer()); } @@ -1114,8 +1116,6 @@ void MIME_Entity::FinishDecodeBase64() if ( base64_decoder->Done(&rlen, &prbuf) ) { // some remaining data - if ( prbuf != rbuf ) - reporter->InternalError("buffer pointer modified in base64 decoding"); if ( rlen > 0 ) DataOctets(rlen, rbuf); } @@ -1390,7 +1390,11 @@ void MIME_Mail::SubmitAllHeaders(MIME_HeaderList& hlist) void MIME_Mail::SubmitData(int len, const char* buf) { if ( buf != (char*) data_buffer->Bytes() + buffer_start ) - reporter->InternalError("buffer misalignment"); + { + reporter->AnalyzerError(GetAnalyzer(), + "MIME buffer misalignment"); + return; + } if ( compute_content_hash ) { @@ -1483,7 +1487,9 @@ void MIME_Mail::SubmitEvent(int event_type, const char* detail) break; default: - reporter->InternalError("unrecognized MIME_Mail event"); + reporter->AnalyzerError(GetAnalyzer(), + "unrecognized MIME_Mail event"); + return; } if ( mime_event ) diff --git a/src/analyzer/protocol/mime/MIME.h b/src/analyzer/protocol/mime/MIME.h index 8d83609cc1..2b2f88105d 100644 --- a/src/analyzer/protocol/mime/MIME.h +++ b/src/analyzer/protocol/mime/MIME.h @@ -193,7 +193,8 @@ public: virtual ~MIME_Message() { if ( ! finished ) - reporter->InternalError("Done() must be called before destruction"); + reporter->AnalyzerError(analyzer, + "missing MIME_Message::Done() call"); } virtual void Done() { finished = 1; } diff --git a/src/analyzer/protocol/netflow/netflow-analyzer.pac b/src/analyzer/protocol/netflow/netflow-analyzer.pac index 666de076c8..439215dbc1 100644 --- a/src/analyzer/protocol/netflow/netflow-analyzer.pac +++ b/src/analyzer/protocol/netflow/netflow-analyzer.pac @@ -31,8 +31,11 @@ flow NetFlow_Flow { internal_type("nf_v5_record")->AsRecordType(); nfheader_id_type = internal_type("nfheader_id")->AsRecordType(); - pdu_id = 0; identifier = NULL; + exporter_ip = 0; + uptime = 0; + export_time = 0; + pdu_id = 0; %} # %cleanup does not only put the cleanup code into the destructor, diff --git a/src/analyzer/protocol/pop3/POP3.cc b/src/analyzer/protocol/pop3/POP3.cc index 52f9eb8445..388a055ee2 100644 --- a/src/analyzer/protocol/pop3/POP3.cc +++ b/src/analyzer/protocol/pop3/POP3.cc @@ -209,7 +209,10 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line) break; default: - reporter->InternalError("unexpected authorization state"); + reporter->AnalyzerError(this, + "unexpected POP3 authorization state"); + delete decoded; + return; } delete decoded; @@ -565,7 +568,8 @@ void POP3_Analyzer::ProcessClientCmd() break; default: - reporter->InternalError("command not known"); + reporter->AnalyzerError(this, "unknown POP3 command"); + return; } } diff --git a/src/analyzer/protocol/rpc/RPC.cc b/src/analyzer/protocol/rpc/RPC.cc index fd76bf551b..1ffc7dd26e 100644 --- a/src/analyzer/protocol/rpc/RPC.cc +++ b/src/analyzer/protocol/rpc/RPC.cc @@ -265,7 +265,10 @@ int RPC_Interpreter::DeliverRPC(const u_char* buf, int n, int rpclen, } else if ( n < 0 ) - reporter->InternalError("RPC underflow"); + { + reporter->AnalyzerError(analyzer, "RPC underflow"); + return 0; + } return 1; } @@ -474,8 +477,12 @@ bool Contents_RPC::CheckResync(int& len, const u_char*& data, bool orig) } if ( resync_toskip != 0 ) + { // Should never happen. - reporter->InternalError("RPC resync: skipping over data failed"); + reporter->AnalyzerError(this, + "RPC resync: skipping over data failed"); + return false; + } // Now lets see whether data points to the beginning of a RPC // frame. If the resync processs is successful, we should be @@ -625,7 +632,11 @@ void Contents_RPC::DeliverStream(int len, const u_char* data, bool orig) marker_buf.Init(4,4); if ( ! dummy_p ) - reporter->InternalError("inconsistent RPC record marker extraction"); + { + reporter->AnalyzerError(this, + "inconsistent RPC record marker extraction"); + return; + } last_frag = (marker & 0x80000000) != 0; marker &= 0x7fffffff; diff --git a/src/analyzer/protocol/smb/SMB.cc b/src/analyzer/protocol/smb/SMB.cc index 4393626217..9d388a0886 100644 --- a/src/analyzer/protocol/smb/SMB.cc +++ b/src/analyzer/protocol/smb/SMB.cc @@ -738,26 +738,18 @@ int SMB_Session::ParseTransaction(int is_orig, int cmd, break; default: - reporter->InternalError("command mismatch for ParseTransaction"); + reporter->AnalyzerError(analyzer, + "command mismatch for SMB_Session::ParseTransaction"); + return 0; } - int ret; - if ( is_orig ) - { - if ( cmd == SMB_COM_TRANSACTION || cmd == SMB_COM_TRANSACTION2 ) - ret = ParseTransactionRequest(cmd, hdr, body); + if ( ! is_orig ) + return ParseTransactionResponse(cmd, hdr, body); - else if ( cmd == SMB_COM_TRANSACTION_SECONDARY || - cmd == SMB_COM_TRANSACTION2_SECONDARY ) - ret = ParseTransactionSecondaryRequest(cmd, hdr, body); + if ( cmd == SMB_COM_TRANSACTION || cmd == SMB_COM_TRANSACTION2 ) + return ParseTransactionRequest(cmd, hdr, body); - else - ret = 0; - } - else - ret = ParseTransactionResponse(cmd, hdr, body); - - return ret; + return ParseTransactionSecondaryRequest(cmd, hdr, body); } int SMB_Session::ParseTransactionRequest(int cmd, diff --git a/src/analyzer/protocol/tcp/ContentLine.cc b/src/analyzer/protocol/tcp/ContentLine.cc index 2a810c5dd1..63a3c07f00 100644 --- a/src/analyzer/protocol/tcp/ContentLine.cc +++ b/src/analyzer/protocol/tcp/ContentLine.cc @@ -102,9 +102,6 @@ void ContentLine_Analyzer::DeliverStream(int len, const u_char* data, delete [] buf; buf = tmp; - - if ( ! buf ) - reporter->InternalError("out of memory delivering endpoint line"); } DoDeliver(len, data); @@ -126,7 +123,11 @@ void ContentLine_Analyzer::EndpointEOF(bool is_orig) void ContentLine_Analyzer::SetPlainDelivery(int64_t length) { if ( length < 0 ) - reporter->InternalError("negative length for plain delivery"); + { + reporter->AnalyzerError(this, + "negative length for plain delivery"); + return; + } plain_delivery_length = length; } diff --git a/src/analyzer/protocol/tcp/TCP.cc b/src/analyzer/protocol/tcp/TCP.cc index 0c06d7fbd7..aefc5a1808 100644 --- a/src/analyzer/protocol/tcp/TCP.cc +++ b/src/analyzer/protocol/tcp/TCP.cc @@ -1580,7 +1580,9 @@ BroFile* TCP_Analyzer::GetContentsFile(unsigned int direction) const default: break; } - reporter->InternalError("inconsistency in TCP_Analyzer::GetContentsFile"); + + reporter->Error("bad direction %u in TCP_Analyzer::GetContentsFile", + direction); return 0; } diff --git a/src/analyzer/protocol/tcp/TCP_Endpoint.cc b/src/analyzer/protocol/tcp/TCP_Endpoint.cc index 144d4598dd..d596234021 100644 --- a/src/analyzer/protocol/tcp/TCP_Endpoint.cc +++ b/src/analyzer/protocol/tcp/TCP_Endpoint.cc @@ -34,10 +34,8 @@ TCP_Endpoint::TCP_Endpoint(TCP_Analyzer* arg_analyzer, int arg_is_orig) hist_last_SYN = hist_last_FIN = hist_last_RST = 0; - src_addr = is_orig ? tcp_analyzer->Conn()->RespAddr() : - tcp_analyzer->Conn()->OrigAddr(); - dst_addr = is_orig ? tcp_analyzer->Conn()->OrigAddr() : - tcp_analyzer->Conn()->RespAddr(); + src_addr = is_orig ? Conn()->RespAddr() : Conn()->OrigAddr(); + dst_addr = is_orig ? Conn()->OrigAddr() : Conn()->RespAddr(); checksum_base = ones_complement_checksum(src_addr, 0); checksum_base = ones_complement_checksum(dst_addr, checksum_base); @@ -54,6 +52,11 @@ TCP_Endpoint::~TCP_Endpoint() Unref(contents_file); } +Connection* TCP_Endpoint::Conn() const + { + return tcp_analyzer->Conn(); + } + void TCP_Endpoint::Done() { if ( contents_processor ) @@ -143,7 +146,7 @@ void TCP_Endpoint::SetState(EndpointState new_state) // handshake. if ( ! is_handshake(new_state) ) if ( is_handshake(state) && is_handshake(peer->state) ) - tcp_analyzer->Conn()->SetInactivityTimeout(tcp_inactivity_timeout); + Conn()->SetInactivityTimeout(tcp_inactivity_timeout); prev_state = state; state = new_state; @@ -207,8 +210,20 @@ int TCP_Endpoint::DataSent(double t, int seq, int len, int caplen, FILE* f = contents_file->Seek(seq - contents_start_seq); if ( fwrite(data, 1, len, f) < unsigned(len) ) - // ### this should really generate an event - reporter->InternalError("contents write failed"); + { + char buf[256]; + strerror_r(errno, buf, sizeof(buf)); + reporter->Error("TCP contents write failed: %s", buf); + + if ( contents_file_write_failure ) + { + val_list* vl = new val_list(); + vl->append(Conn()->BuildConnVal()); + vl->append(new Val(IsOrig(), TYPE_BOOL)); + vl->append(new StringVal(buf)); + tcp_analyzer->ConnectionEvent(contents_file_write_failure, vl); + } + } } return status; @@ -241,7 +256,7 @@ int TCP_Endpoint::CheckHistory(uint32 mask, char code) code = tolower(code); } - return tcp_analyzer->Conn()->CheckHistory(mask, code); + return Conn()->CheckHistory(mask, code); } void TCP_Endpoint::AddHistory(char code) @@ -249,6 +264,6 @@ void TCP_Endpoint::AddHistory(char code) if ( ! IsOrig() ) code = tolower(code); - tcp_analyzer->Conn()->AddHistory(code); + Conn()->AddHistory(code); } diff --git a/src/analyzer/protocol/tcp/TCP_Reassembler.cc b/src/analyzer/protocol/tcp/TCP_Reassembler.cc index 949be9f599..a1e20dc0e6 100644 --- a/src/analyzer/protocol/tcp/TCP_Reassembler.cc +++ b/src/analyzer/protocol/tcp/TCP_Reassembler.cc @@ -174,7 +174,8 @@ void TCP_Reassembler::Undelivered(int up_to_seq) } if ( seq_delta(up_to_seq, last_reassem_seq) <= 0 ) - // This should never happen. + // This should never happen. (Reassembler::TrimToSeq has the only call + // to this method and only if this condition is not true). reporter->InternalError("Calling Undelivered for data that has already been delivered (or has already been marked as undelivered"); if ( last_reassem_seq == 1 && @@ -322,17 +323,36 @@ void TCP_Reassembler::RecordToSeq(int start_seq, int stop_seq, BroFile* f) void TCP_Reassembler::RecordBlock(DataBlock* b, BroFile* f) { - unsigned int len = b->Size(); - if ( ! f->Write((const char*) b->block, len) ) - // ### this should really generate an event - reporter->InternalError("contents write failed"); + if ( f->Write((const char*) b->block, b->Size()) ) + return; + + reporter->Error("TCP_Reassembler contents write failed"); + + if ( contents_file_write_failure ) + { + val_list* vl = new val_list(); + vl->append(Endpoint()->Conn()->BuildConnVal()); + vl->append(new Val(IsOrig(), TYPE_BOOL)); + vl->append(new StringVal("TCP reassembler content write failure")); + tcp_analyzer->ConnectionEvent(contents_file_write_failure, vl); + } } void TCP_Reassembler::RecordGap(int start_seq, int upper_seq, BroFile* f) { - if ( ! f->Write(fmt("\n<>\n", seq_delta(upper_seq, start_seq))) ) - // ### this should really generate an event - reporter->InternalError("contents gap write failed"); + if ( f->Write(fmt("\n<>\n", seq_delta(upper_seq, start_seq))) ) + return; + + reporter->Error("TCP_Reassembler contents gap write failed"); + + if ( contents_file_write_failure ) + { + val_list* vl = new val_list(); + vl->append(Endpoint()->Conn()->BuildConnVal()); + vl->append(new Val(IsOrig(), TYPE_BOOL)); + vl->append(new StringVal("TCP reassembler gap write failure")); + tcp_analyzer->ConnectionEvent(contents_file_write_failure, vl); + } } void TCP_Reassembler::BlockInserted(DataBlock* start_block) diff --git a/src/analyzer/protocol/tcp/events.bif b/src/analyzer/protocol/tcp/events.bif index af61783ac4..216691dea1 100644 --- a/src/analyzer/protocol/tcp/events.bif +++ b/src/analyzer/protocol/tcp/events.bif @@ -287,3 +287,13 @@ event tcp_contents%(c: connection, is_orig: bool, seq: count, contents: string%) ## TODO. event tcp_rexmit%(c: connection, is_orig: bool, seq: count, len: count, data_in_flight: count, window: count%); +## Generated when failing to write contents of a TCP stream to a file. +## +## c: The connection whose contents are being recorded. +## +## is_orig: Which side of the connection encountered a failure to write. +## +## msg: A reason or description for the failure. +## +## .. bro:see:: set_contents_file get_contents_file +event contents_file_write_failure%(c: connection, is_orig: bool, msg: string%); diff --git a/src/analyzer/protocol/tcp/functions.bif b/src/analyzer/protocol/tcp/functions.bif index ff812b80ee..f5b0033ae8 100644 --- a/src/analyzer/protocol/tcp/functions.bif +++ b/src/analyzer/protocol/tcp/functions.bif @@ -114,7 +114,7 @@ function get_gap_summary%(%): gap_info ## missing data; this can happen, e.g., due to an ## :bro:id:`ack_above_hole` event. ## -## .. bro:see:: get_contents_file set_record_packets +## .. bro:see:: get_contents_file set_record_packets contents_file_write_failure function set_contents_file%(cid: conn_id, direction: count, f: file%): bool %{ Connection* c = sessions->FindConnection(cid); @@ -137,7 +137,7 @@ function set_contents_file%(cid: conn_id, direction: count, f: file%): bool ## but there is no contents file for *direction*, then the function ## generates an error and returns a file handle to ``stderr``. ## -## .. bro:see:: set_contents_file set_record_packets +## .. bro:see:: set_contents_file set_record_packets contents_file_write_failure function get_contents_file%(cid: conn_id, direction: count%): file %{ Connection* c = sessions->FindConnection(cid); diff --git a/src/file_analysis/Manager.cc b/src/file_analysis/Manager.cc index 8dfb220381..0337dbb098 100644 --- a/src/file_analysis/Manager.cc +++ b/src/file_analysis/Manager.cc @@ -362,12 +362,19 @@ Analyzer* Manager::InstantiateAnalyzer(Tag tag, RecordVal* args, File* f) const Component* c = Lookup(tag); if ( ! c ) - reporter->InternalError("cannot instantiate unknown file analyzer: %s", - tag.AsString().c_str()); + { + reporter->InternalWarning( + "unknown file analyzer instantiation request: %s", + tag.AsString().c_str()); + return 0; + } if ( ! c->Factory() ) - reporter->InternalError("file analyzer %s cannot be instantiated " + { + reporter->InternalWarning("file analyzer %s cannot be instantiated " "dynamically", c->CanonicalName()); + return 0; + } return c->Factory()(args, f); } diff --git a/src/input/Manager.cc b/src/input/Manager.cc index cd02db06af..ee1e1ef522 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -42,6 +42,14 @@ ReaderDefinition input_readers[] = { { BifEnum::Input::READER_DEFAULT, "None", 0, (ReaderBackend* (*)(ReaderFrontend* frontend))0 } }; +static void delete_value_ptr_array(Value** vals, int num_fields) + { + for ( int i = 0; i < num_fields; ++i ) + delete vals[i]; + + delete [] vals; + } + /** * InputHashes are used as Dictionaries to store the value and index hashes * for all lines currently stored in a table. Index hash is stored as @@ -330,7 +338,9 @@ bool Manager::CreateStream(Stream* info, RecordVal* description) break; default: - reporter->InternalError("unknown reader mode"); + reporter->InternalWarning("unknown input reader mode"); + Unref(mode); + return false; } Unref(mode); @@ -1014,7 +1024,8 @@ void Manager::SendEntry(ReaderFrontend* reader, Value* *vals) Stream *i = FindStream(reader); if ( i == 0 ) { - reporter->InternalError("Unknown reader in SendEntry"); + reporter->InternalWarning("Unknown reader %s in SendEntry", + reader->Name()); return; } @@ -1041,10 +1052,7 @@ void Manager::SendEntry(ReaderFrontend* reader, Value* *vals) else assert(false); - for ( int i = 0; i < readFields; i++ ) - delete vals[i]; - - delete [] vals; + delete_value_ptr_array(vals, readFields); } int Manager::SendEntryTable(Stream* i, const Value* const *vals) @@ -1242,7 +1250,8 @@ void Manager::EndCurrentSend(ReaderFrontend* reader) if ( i == 0 ) { - reporter->InternalError("Unknown reader in EndCurrentSend"); + reporter->InternalWarning("Unknown reader %s in EndCurrentSend", + reader->Name()); return; } @@ -1352,7 +1361,8 @@ void Manager::SendEndOfData(ReaderFrontend* reader) if ( i == 0 ) { - reporter->InternalError("Unknown reader in SendEndOfData"); + reporter->InternalWarning("Unknown reader %s in SendEndOfData", + reader->Name()); return; } @@ -1378,7 +1388,7 @@ void Manager::Put(ReaderFrontend* reader, Value* *vals) Stream *i = FindStream(reader); if ( i == 0 ) { - reporter->InternalError("Unknown reader in Put"); + reporter->InternalWarning("Unknown reader %s in Put", reader->Name()); return; } @@ -1410,10 +1420,7 @@ void Manager::Put(ReaderFrontend* reader, Value* *vals) else assert(false); - for ( int i = 0; i < readFields; i++ ) - delete vals[i]; - - delete [] vals; + delete_value_ptr_array(vals, readFields); } int Manager::SendEventStreamEvent(Stream* i, EnumVal* type, const Value* const *vals) @@ -1511,7 +1518,6 @@ int Manager::PutTable(Stream* i, const Value* const *vals) EnumVal* ev; int startpos = 0; Val* predidx = ValueToRecordVal(vals, stream->itype, &startpos); - Ref(valval); if ( updated ) ev = new EnumVal(BifEnum::Input::EVENT_CHANGED, @@ -1522,7 +1528,10 @@ int Manager::PutTable(Stream* i, const Value* const *vals) bool result; if ( stream->num_val_fields > 0 ) // we have values + { + Ref(valval); result = CallPred(stream->pred, 3, ev, predidx, valval); + } else // no values result = CallPred(stream->pred, 2, ev, predidx); @@ -1585,7 +1594,8 @@ void Manager::Clear(ReaderFrontend* reader) Stream *i = FindStream(reader); if ( i == 0 ) { - reporter->InternalError("Unknown reader in Clear"); + reporter->InternalWarning("Unknown reader %s in Clear", + reader->Name()); return; } @@ -1606,7 +1616,7 @@ bool Manager::Delete(ReaderFrontend* reader, Value* *vals) Stream *i = FindStream(reader); if ( i == 0 ) { - reporter->InternalError("Unknown reader in Delete"); + reporter->InternalWarning("Unknown reader %s in Delete", reader->Name()); return false; } @@ -1686,11 +1696,7 @@ bool Manager::Delete(ReaderFrontend* reader, Value* *vals) return false; } - for ( int i = 0; i < readVals; i++ ) - delete vals[i]; - - delete [] vals; - + delete_value_ptr_array(vals, readVals); return success; } @@ -1722,6 +1728,7 @@ bool Manager::SendEvent(const string& name, const int num_vals, Value* *vals) if ( handler == 0 ) { reporter->Error("Event %s not found", name.c_str()); + delete_value_ptr_array(vals, num_vals); return false; } @@ -1735,6 +1742,7 @@ bool Manager::SendEvent(const string& name, const int num_vals, Value* *vals) if ( num_vals != num_event_vals ) { reporter->Error("Wrong number of values for event %s", name.c_str()); + delete_value_ptr_array(vals, num_vals); return false; } @@ -1744,11 +1752,7 @@ bool Manager::SendEvent(const string& name, const int num_vals, Value* *vals) mgr.QueueEvent(handler, vl, SOURCE_LOCAL); - for ( int i = 0; i < num_vals; i++ ) - delete vals[i]; - - delete [] vals; - + delete_value_ptr_array(vals, num_vals); return true; } @@ -1794,12 +1798,6 @@ RecordVal* Manager::ListValToRecordVal(ListVal* list, RecordType *request_type, { assert(position != 0 ); // we need the pointer to point to data; - if ( request_type->Tag() != TYPE_RECORD ) - { - reporter->InternalError("ListValToRecordVal called on non-record-value."); - return 0; - } - RecordVal* rec = new RecordVal(request_type->AsRecordType()); assert(list != 0); @@ -1830,12 +1828,6 @@ RecordVal* Manager::ValueToRecordVal(const Value* const *vals, { assert(position != 0); // we need the pointer to point to data. - if ( request_type->Tag() != TYPE_RECORD ) - { - reporter->InternalError("ValueToRecordVal called on non-record-value."); - return 0; - } - RecordVal* rec = new RecordVal(request_type->AsRecordType()); for ( int i = 0; i < request_type->NumFields(); i++ ) { diff --git a/src/input/readers/Binary.cc b/src/input/readers/Binary.cc index e49e6b7407..96a9028f7b 100644 --- a/src/input/readers/Binary.cc +++ b/src/input/readers/Binary.cc @@ -53,8 +53,8 @@ bool Binary::CloseInput() { if ( ! in || ! in->is_open() ) { - InternalError(Fmt("Trying to close closed file for stream %s", - fname.c_str())); + InternalWarning(Fmt("Trying to close closed file for stream %s", + fname.c_str())); return false; } diff --git a/src/input/readers/Raw.cc b/src/input/readers/Raw.cc index de0a264ec4..d795430ba3 100644 --- a/src/input/readers/Raw.cc +++ b/src/input/readers/Raw.cc @@ -289,7 +289,8 @@ bool Raw::OpenInput() return false; } - fcntl(fileno(file), F_SETFD, FD_CLOEXEC); + if ( ! SetFDFlags(fileno(file), F_SETFD, FD_CLOEXEC) ) + Warning(Fmt("Init: cannot set close-on-exec for %s", fname.c_str())); } return true; @@ -299,7 +300,8 @@ bool Raw::CloseInput() { if ( file == 0 ) { - InternalError(Fmt("Trying to close closed file for stream %s", fname.c_str())); + InternalWarning(Fmt("Trying to close closed file for stream %s", + fname.c_str())); return false; } #ifdef DEBUG @@ -492,9 +494,6 @@ int64_t Raw::GetLine(FILE* arg_file) Error(Fmt("Reader encountered unexpected error code %d", errno)); return -3; } - - InternalError("Internal control flow execution error in raw reader"); - assert(false); } // write to the stdin of the child process diff --git a/src/plugin/Component.cc b/src/plugin/Component.cc index 074f00c264..ee18d55cdf 100644 --- a/src/plugin/Component.cc +++ b/src/plugin/Component.cc @@ -44,7 +44,9 @@ void Component::Describe(ODesc* d) const break; default: - reporter->InternalError("unknown component type in plugin::Component::Describe"); + reporter->InternalWarning("unknown component type in plugin::Component::Describe"); + d->Add(""); + break; } d->Add("]"); diff --git a/src/plugin/ComponentManager.h b/src/plugin/ComponentManager.h index 16f9d80743..ccc076db28 100644 --- a/src/plugin/ComponentManager.h +++ b/src/plugin/ComponentManager.h @@ -169,11 +169,12 @@ const char* ComponentManager::GetComponentName(T tag) const C* c = Lookup(tag); - if ( ! c ) - reporter->InternalError("request for name of unknown component tag %s", - tag.AsString().c_str()); + if ( c ) + return c->CanonicalName(); - return c->CanonicalName(); + reporter->InternalWarning("requested name of unknown component tag %s", + tag.AsString().c_str()); + return error; } template diff --git a/src/threading/SerialTypes.cc b/src/threading/SerialTypes.cc index c0e26ccb32..a4f76a9b2d 100644 --- a/src/threading/SerialTypes.cc +++ b/src/threading/SerialTypes.cc @@ -294,7 +294,8 @@ bool Value::Read(SerializationFormat* fmt) } default: - reporter->InternalError("unsupported type %s in Value::Write", type_name(type)); + reporter->InternalError("unsupported type %s in Value::Read", + type_name(type)); } return false; @@ -398,7 +399,8 @@ bool Value::Write(SerializationFormat* fmt) const } default: - reporter->InternalError("unsupported type %s in Value::REad", type_name(type)); + reporter->InternalError("unsupported type %s in Value::Write", + type_name(type)); } return false; diff --git a/src/util.cc b/src/util.cc index a2a21c7bc6..dd232a83fa 100644 --- a/src/util.cc +++ b/src/util.cc @@ -345,9 +345,9 @@ unsigned char encode_hex(int h) '9', 'A', 'B', 'C', 'D', 'E', 'F' }; - if ( h < 0 || h >= 16 ) + if ( h < 0 || h > 15 ) { - reporter->InternalError("illegal value for encode_hex: %d", h); + reporter->InternalWarning("illegal value for encode_hex: %d", h); return 'X'; } @@ -652,16 +652,8 @@ void hmac_md5(size_t size, const unsigned char* bytes, unsigned char digest[16]) static bool read_random_seeds(const char* read_file, uint32* seed, uint32* buf, int bufsiz) { - struct stat st; FILE* f = 0; - if ( stat(read_file, &st) < 0 ) - { - reporter->Warning("Seed file '%s' does not exist: %s", - read_file, strerror(errno)); - return false; - } - if ( ! (f = fopen(read_file, "r")) ) { reporter->Warning("Could not open seed file '%s': %s", diff --git a/testing/btest/scripts/base/frameworks/input/sqlite/basic.bro b/testing/btest/scripts/base/frameworks/input/sqlite/basic.bro index 03bc91beb7..aa3a75ae4b 100644 --- a/testing/btest/scripts/base/frameworks/input/sqlite/basic.bro +++ b/testing/btest/scripts/base/frameworks/input/sqlite/basic.bro @@ -1,6 +1,8 @@ # # @TEST-GROUP: sqlite # +# @TEST-REQUIRES: which sqlite3 +# # @TEST-EXEC: cat conn.sql | sqlite3 conn.sqlite # @TEST-EXEC: btest-bg-run bro bro -b %INPUT # @TEST-EXEC: btest-bg-wait 10 diff --git a/testing/btest/scripts/base/frameworks/input/sqlite/error.bro b/testing/btest/scripts/base/frameworks/input/sqlite/error.bro index 53ac1e0863..c6712df99a 100644 --- a/testing/btest/scripts/base/frameworks/input/sqlite/error.bro +++ b/testing/btest/scripts/base/frameworks/input/sqlite/error.bro @@ -1,3 +1,5 @@ +# @TEST-REQUIRES: which sqlite3 +# # @TEST-EXEC: cat ssh.sql | sqlite3 ssh.sqlite # # @TEST-GROUP: sqlite diff --git a/testing/btest/scripts/base/frameworks/input/sqlite/port.bro b/testing/btest/scripts/base/frameworks/input/sqlite/port.bro index 049ad2a386..c2949b5b3e 100644 --- a/testing/btest/scripts/base/frameworks/input/sqlite/port.bro +++ b/testing/btest/scripts/base/frameworks/input/sqlite/port.bro @@ -1,6 +1,8 @@ # # @TEST-GROUP: sqlite # +# @TEST-REQUIRES: which sqlite3 +# # @TEST-EXEC: cat port.sql | sqlite3 port.sqlite # @TEST-EXEC: btest-bg-run bro bro -b %INPUT # @TEST-EXEC: btest-bg-wait 10 diff --git a/testing/btest/scripts/base/frameworks/input/sqlite/types.bro b/testing/btest/scripts/base/frameworks/input/sqlite/types.bro index 4e60de3a96..ddcbefa67f 100644 --- a/testing/btest/scripts/base/frameworks/input/sqlite/types.bro +++ b/testing/btest/scripts/base/frameworks/input/sqlite/types.bro @@ -1,3 +1,5 @@ +# @TEST-REQUIRES: which sqlite3 +# # @TEST-EXEC: cat ssh.sql | sqlite3 ssh.sqlite # # @TEST-GROUP: sqlite diff --git a/testing/btest/scripts/base/frameworks/logging/sqlite/error.bro b/testing/btest/scripts/base/frameworks/logging/sqlite/error.bro index 27193250a4..2e5d22f188 100644 --- a/testing/btest/scripts/base/frameworks/logging/sqlite/error.bro +++ b/testing/btest/scripts/base/frameworks/logging/sqlite/error.bro @@ -1,4 +1,5 @@ # +# @TEST-REQUIRES: which sqlite3 # @TEST-REQUIRES: has-writer SQLite # @TEST-GROUP: sqlite # diff --git a/testing/btest/scripts/base/frameworks/logging/sqlite/types.bro b/testing/btest/scripts/base/frameworks/logging/sqlite/types.bro index a6a1c04b02..7c896a7192 100644 --- a/testing/btest/scripts/base/frameworks/logging/sqlite/types.bro +++ b/testing/btest/scripts/base/frameworks/logging/sqlite/types.bro @@ -1,4 +1,5 @@ # +# @TEST-REQUIRES: which sqlite3 # @TEST-REQUIRES: has-writer SQLite # @TEST-GROUP: sqlite # diff --git a/testing/btest/scripts/base/frameworks/logging/sqlite/wikipedia.bro b/testing/btest/scripts/base/frameworks/logging/sqlite/wikipedia.bro index 0e40c60008..b48520440a 100644 --- a/testing/btest/scripts/base/frameworks/logging/sqlite/wikipedia.bro +++ b/testing/btest/scripts/base/frameworks/logging/sqlite/wikipedia.bro @@ -1,4 +1,5 @@ # +# @TEST-REQUIRES: which sqlite3 # @TEST-REQUIRES: has-writer SQLite # @TEST-GROUP: sqlite # diff --git a/testing/btest/scripts/base/utils/active-http.test b/testing/btest/scripts/base/utils/active-http.test index f547e7dd15..377c0dfd7b 100644 --- a/testing/btest/scripts/base/utils/active-http.test +++ b/testing/btest/scripts/base/utils/active-http.test @@ -1,4 +1,5 @@ # @TEST-REQUIRES: which python +# @TEST-REQUIRES: which curl # # @TEST-EXEC: btest-bg-run httpd python $SCRIPTS/httpd.py --max 1 # @TEST-EXEC: sleep 3