In deterministic mode, RAND_MAX is not related to the result of
zeek::random_number() (formerly bro_random()), but some logic was
using RAND_MAX as indication of the possible range of values. The
new zeek::max_random() will give the correct upper-bound regardless
of whether deterministic-mode is used.
The type used for storing the state of the RNG is changed from
`unsigned int` to `long int` since the former has a minimal range
of [0, 65,535] while the RNG function itself has a range of
[1, 2147483646]. A `long int` must be capable of
[−2147483647, +2147483647] and is also the return type of `random()`,
which is what zeek::prng() aims to roughly parity.
The bro_prng() implementation cannot generate 0 as a result since it
causes every subsequent number from the PRNG to also be 0, so use the
number 1 instead of 0.
The intermediate result of the PRNG used unsigned storage, preventing
the ( result < 0 ) branch from ever being evaluated. This could cause
return values to exceed the modulus as well as RAND_MAX.
One interesting effect of this is potential for the rand() BIF to
return values outside the requested maximum limit.
Another interesting effect of this is that a PacketFilter may start
randomly dropping packets even if it was not configured for
random-packet-drops.
* origin/topic/johanna/table-changes: (26 commits)
TableSync: try to make test more robust & add debug output
Increase timeouts to see if FreeBSD will be happy with this.
Try to make FreeBSD test happy with larger timeout.
TableSync: refactor common functionality into function
TableSync: don't raise &on_change, smaller fixes
TableSync: rename auto_store -> table_store
SyncTables: address feedback part 1 - naming (broker and zeek)
BrokerStore <-> Zeek Tables: cleanup and bug workaround
Zeek Table<->Brokerstore: cleanup, documentation, small fixes
BrokerStore<->Zeek table: adopt to recent Zeek API changes
BrokerStore<->Zeek Tables Fix a few small test failures.
BrokerStore<->Zeek tables: allow setting storage location & tests
BrokerStore<->Zeek tables: &backend works for in-memory stores.
BrokerStore<->Zeek table - introdude &backend attribute
BrokerStore<->Zeek tables: test for clones synchronizing to a master
BrokerStore<->Zeek tables: load persistent tables on startup.
Brokerstore<->Tables: attribute conflicts
Zeek/Brokerstore updates: expiration
Zeek/Brokerstore updates: add test that includes updates from clones
Zeek/Brokerstore updates: first working end-to-end test
...
The Contents_NetbiosSSN analyzer used a recursive message parsing
function that determined the size of the next message from the input
packet-data itself. A packet containing a sequence of many small
messages could cause a stack overflow since a recursion happened after
processing each message.
The Contents_DNS analyzer used a recursive message parsing function that
determined the size of the next message from the input packet-data
itself. A packet containing a sequence of many small messages could
cause a stack overflow since a recursion happenened after processing
each message.
Credit to OSS-Fuzz for discovery
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=24272
* origin/topic/timw/deprecation-fixes-for-spicy:
Use namespaced version of Location to silence warnings
Various deprecation fixes, reported by failed Spicy builds
Add deprecated version of EnumType::GetVal() to returns EnumVal*, rename IntrusivePtr version to GetEnumVal
Merge adjustments:
- Revert unneeded changes in mmdb_dir lookups to fix build
- Fix deprecated EnumType::GetVal() to return +1 ref-count
* origin/topic/timw/deprecation-fixes-for-spicy:
Various deprecation fixes, reported by failed Spicy builds
Add deprecated version of EnumType::GetVal() to returns EnumVal*, rename IntrusivePtr version to GetEnumVal
- Add deprecated version of ID::SetType() that takes Type*
- Add deprecated versions of zeek::set_location in the global namespace
- Fix global namespace version of lookup_ID to return ID*
This addresses PR feedback. The main component in this commit is to
disable &on_change notifications when &backend loads a table from sqlite
on startup.
- Add deprecated version of ID::SetType() that takes Type*
- Add deprecated versions of zeek::set_location in the global namespace
- Fix global namespace version of lookup_ID to return ID*
* origin/topic/jsiwek/gh-1024-broker-store-handle-type-checks:
Improve Broker store API's handling of invalid arguments
Add builtin_exception() functions
GH-1024: fix crash on passing wrong types to Broker store API
This was a bit of a bigger merge since Zeek changed inbetween the time
of the PR and me actually merging it.
I put the new functions into the zeek::detail namespace -- since it
seems unlikely that those will be used by something external.
I also renamed them to fit better with the naming scheme of the new
error functions.
Fixes GH-1024
* origin/topic/jsiwek/deprecation-improvements:
Fix wrong frame offsets for locals of alternate event/hook prototypes
Add deprecation expression to deprecated prototype/parameter messages
Improve "use of deprecated prototype" warning message
Emit deprecation warning for use of &deprecated function parameters
This commit adds script/c++ documentation and fixes a few loose ends.
It also adds tests for corner cases and massively improves error
messages.
This also actually introduces type-compatibility checking and introduces
a new attribute that lets a user override this if they really know what
they are doing. I am not quite sure if we should really let that stay in
- but it can be very convenient to have this functionality.
One test is continuing to fail - the expiry test is very flaky. This is,
I think, caused by delays of the broker store forwarding. I am unsure if
we can actually do anything about that.
Previously, a single `icmp_conn` record was built per ICMP "connection"
and re-used for all events generated from it. This may have been a
historical attempt at performance optimization, but:
* By default, Zeek does not load any scripts that handle ICMP events.
* The one script Zeek ships with that does handle ICMP events,
"detect-traceroute", is already noted as being disabled due to
potential performance problems of doing that kind of analysis.
* Re-use of the original `icmp_conn` record tends to misreport
TTL and length values since they come from original packet instead
of the current one.
* Even if we chose to still re-use `icmp_conn` records and just fill
in a new TTL and length value each packet, a user script could have
stored a reference to the record and not be expecting those values
to be changed out from underneath them.
Now, a new `icmp_info` record is created/populated in all ICMP events
and should be used instead of `icmp_conn`. It also removes the
orig_h/resp_h fields as those are redundant with what's already
available in the connection record.
Local frame offsets were being assigned based on number of the alternate
prototype's parameters, which may end up having less total parameters
than the canonical prototype, causing the local value to incorrectly
overwrite an event/hook argument value.
The location information now points out the place of the deprecated
prototype instead of the location where the ID was initially declared
(which may not itself be a deprecated prototype).
Particularly, this is meant for using &deprecated on canonical
event/hook prototype parameters to encourage users to create handlers
to another, non-deprecated prototype. i.e. for canonical prototypes,
we may not always want to put &deprecated directly on the prototype
itself since that signals deprecation of the ID entirely.
