Script and BIF functions with a single any parameter are excluded from
type checking regarding arguments. This makes it possible to call a
ScriptFunc with more arguments than it actually has parameters and frame
space for, causing heap-buffer-overflows.
This change runtime checks expected parameters and provided arguments
and short-circuits execution as well as logging runtime expression errors.
Fixes#2446
Seemed easiest to do it via the traversal infrastructure as we do not
otherwise track enough context/scope when instantiating break or next
statements.
Might be worth moving this out of src/parse.y, but didn't exactly know
where. Or maybe we wait until there's more such trivial validations
popping up
Fixes#2440
Using positional and vararg arguments for BIFs, it's not possible to do
proper runtime type checking on them as discussed in #2425. The bifcl produced
code unconditionally attempts to convert the positional arguments to StringVals,
but nothing ever type checks them. Instead of improving the vararg support in
Zeek script and bifcl, align cat_sep() with fmt() in making it fully vararg
and do implement type checks by hand.
With this change, passing wrong types for the separator and default argument
isn't a fatal error anymore and the error messages are also more descriptive.
It's a bit of a crutch working around varargs limitations.
Fixes#2425
One more from @stevesmoot. The record_fields() BIF produced "enum" as
type_name for fields of type enum.
Extend container_type_name() to append the actual name of the enum.
This is changing the format and may break consumers, but those are
likely in a category that are happy to adapt. Not having the actual
enum name available wasn't very helpful.
We could alternatively render only the actual type_name without the
prefixed "enum", but that isn't how it's done for record types currently
and it would make it more difficult to decide which subsequent BIFs to
use for further introspection, like enum_names().
This started with reverting commit 52cd02173d
and then rewriting it to be per handler rather than handler identifier
and adding support for hooks as well as adding implicit module groups.
This seems to be an age-old bug. Reported by mchen on discourse [1].
The TCPSessionAdapter decides in AddExtraAnalyzers() whether to enable
reassembly or not. When dpd_reassemble_first_packets is F, this boils down to
! GetChildren().empty(). The intention being that if any analyzers have been
added to the connection based on known ports, reassembly is to be enabled.
However, GetChildren() does not take into account new_children and so
! GetChildren().empty() is always false here and reassembly solely
based on dpd_reassemble_first_packets=F (or the tcp_content... options).
Ouch.
Call AppendNewChildren() before AddExtraAnalyzers() as a fix. Without this,
the new test does not produce an http.log and service "http" isn't in conn.log.
[1] https://community.zeek.org/t/how-to-activate-an-application-layer-analyzer-when-signature-dpd-reassemble-first-packets-is-off/6763
Mostly: Do not instantiate variables within for loops and allow
reusing differently typed blanks which previously wasn't possible.
This may be missing some corner-cases, but the added tests seem
to work as expected and nothing else fell apart it seems.
In his ZeekWeek 2022 presentation, @stevesmoot mentioned that he had a
difficult time looking up enum names when all he had was a string
naming the type.
Add support to enum_names() to transparently lookup the type if a string
is provided. This is similar in how record_fields() behaves when being
passed a string.
This has come up a few times and the motivation is mainly better "first timer"
experience with Zeek. Concretely, if one wants to run a Zeek cluster with
multiple workers and reasonable load balancing on Linux, AF_PACKET is a decent
start. Without AF_PACKET support being built into Zeek, however, a new user's
next experience is that of setting up a development environment in order
to compile an external plugin (think compiler, kernel headers, zkg, ...).
Only to get what could be termed basic functionality.
This is using the ZEEK_INCLUDE_PLUGINS infrastructure. I've used the all
upper case spelling of AF_PACKET in the help output because it seems everyone
else references/writes it like that. I think we should also write it
like that in the docs.
This goes the hard-exit on conflicts route as IMO it provides better
messaging that something is wrong, rather than defaulting to something
the user may not expect.
Fixes#2403
Before, that API was part of the analyzers themselves, which meant we
couldn't disable a packet analyzer before it had been instantiated.
That's different from protocol/file analyzers, where we disable them
through the corresponding component. The lack of the component-side
API prevented Spicy from replacing packet analyzers at startup.
The reason we had put this into analyzer originally was performance so
that we don't need a component lookup for every packet. This change
keeps that optimization by caching the on/off state in the analyzer
itself as well, but now with the component being the one controlling
it.
This uses the v3 json as a source for the first time. The test needed
some updating because Google removed a couple more logs - in the future
this should hopefully not be neccessary anymore because I think v3
should retain all logs.
In theory this might be neat in 5.1.
This allows to enable/disable file analyzers through the same interfaces
as packet and protocol analyzers, specifically Analyzer::disable_analyzer
could be interesting.
This adds machinery to the packet_analysis manager for disabling
and enabling packet analyzers and implements two low-level bifs
to use it.
Extend Analyzer::enable_analyzer() and Analyzer::disable_analyzer()
to transparently work with packet analyzers, too. This also allows
to add packet analyzers to Analyzer::disabled_analyzers.
Add a test parsing a malformed PE file showing that analyzer_violation_info
is raised with the fa_file object set.
It could be interesting to pass through an optional connection if one
exists, but access is provided through f$conns, too.
Introduce two new events for analyzer confirmation and analyzer violation
reporting. The current analyzer_confirmation and analyzer_violation
events assume connection objects and analyzer ids are available which
is not always the case. We're already passing aid=0 for packet analyzers
and there's not currently a way to report violations from file analyzers
using analyzer_violation, for example.
These new events use an extensible Info record approach so that additional
(optional) information can be added later without changing the signature.
It would allow for per analyzer extensions to the info records to pass
analyzer specific info to script land. It's not clear that this would be
a good idea, however.
The previous analyzer_confirmation and analyzer_violation events
continue to exist, but are deprecated and will be removed with Zeek 6.1.
There's a logic error in the packet analyzer's AnalyzerConfirmation()
method that causes analyzer_confirmation() events to be raised for every
packet rather than stopping after the first confirmation which appears to
have been the intention. This affects, for example, VXLAN and Geneve tunnels.
The optional arg_tag parameter was used for short-circuit'ing, but the return
value of GetAnalyzerTag() used for setting the session state causing the
disconnect.
In scenarios where Zeek receives purely tunneled monitoring traffic, this may
result in a non-negligible performance impact.
Somewhat related, ensure the session state is set to violated before
short-circuiting if no analyzer_violations are installed.
Suggesting this as a 5.0.3 candidate.
The current_entity tracking in HTTP assumes that client/server never
send HTTP entities at the same time. The attached pcap (generated
artificially) violates this and triggers:
1663698249.307259 expression error in <...>base/protocols/http/./entities.zeek, line 89: field value missing (HTTP::c$http$current_entity)
For the http-no-crlf test, include weird.log as baseline. Now that weird is
@load'ed from http, it is actually created and seems to make sense
to btest-diff it, too.
...the only known cases where the `-` for `connection$service` was
handled is to skip/ignore these analyzers.
Slight suspicion that join_string_set() should maybe become a bif
now determine_service() runs once for each connection.
Closes#2388
* 'topic/amazingpp/2384-record-deprecation' of github.com:/AmazingPP/zeek:
Replace all the Warning() calls after IsFieldDeprecated() over to Warn()
Fix deprecation not flagged and incorrect line number in record
* origin/topic/vern/cpp-maint-Sep22:
oof more manual fixups
undo inadvertently committed tweak to test
update for btest only run in some environments
btest tweaks for recent changes
updates to notes for compile-to-C++ maintenance
newly-created btest files
baseline updates for -a cpp alternative (compile-to-C++)
tweak btest so it's recognized as a candidate for C++ compilation testing
split basic "int" btests into main part versus now-separate overflow part
fix deprecated "local" scoping in test scripts
annotated scripts to skip when testing compilation-to-C++
C++ script generation fix for lambdas that have identical bodies
fix for C++ scripts that refer to "opaque" types
C++ compilation support for 2-valued vector "for" loops
C++ compilation support for RE /s operator
run-time checking of vector operations for overflows and division-by-zero
error propagation fix to avoid a crash
