| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
|
|
|
|
|
|
| |
Else we assert with:
BUG: unknown expression type range
nft: src/netlink_linearize.c:912: netlink_gen_expr: Assertion `0' failed.
While at it, condense meta and exthdr to reuse the same helper.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Testing for range before evaluation will still crash us later during
netlink linearization, prefixes turn into ranges, symbolic expression
might hide a range/prefix.
So move this after the argument has been evaluated.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
| |
Add two extra tests to exercise netdevice removal path.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Test for
'netfilter: nft_set_pipapo: skip inactive elements during set walk'.
Reported-by: Xingyuan Mo <hdthky0@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Use $NFT (src/nft, in-tree binary), not the one installed by the distro.
Else we may not find newly added bugs unless user did "make install" or
bug has propagated to release.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We need to add a custom destructor for this structure, it
contains the dynamically allocated names.
a:5:55-55: Error: syntax error, unexpected '}', expecting string
policy = { estabQisheestablished : 2m3s, cd : 2m3s, }
==562373==ERROR: LeakSanitizer: detected memory leaks
Indirect leak of 160 byte(s) in 2 object(s) allocated from:
#1 0x5a565b in xmalloc src/utils.c:31:8
#2 0x5a565b in xzalloc src/utils.c:70:8
#3 0x3d9352 in nft_parse_bison_filename src/libnftables.c:520:8
[..]
Fixes: c7c94802679c ("src: add ct timeout support")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
netlink_linearize.c has never supported more than 16 chained binops.
Adding more is possible but overwrites the stack in
netlink_gen_bitwise().
Add a recursion counter to catch this at eval stage.
Its not enough to just abort once the counter hits
NFT_MAX_EXPR_RECURSION.
This is because there are valid test cases that exceed this.
For example, evaluation of 1 | 2 will merge the constans, so even
if there are a dozen recursive eval calls this will not end up
with large binop chain post-evaluation.
v2: allow more than 16 binops iff the evaluation function
did constant-merging.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
Byteorder switch in this function may undersize the conversion
buffer by one byte, this needs to use div_round_up().
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
BUG: Value export of 512 bytes would overflownft: src/netlink.c:474: netlink_gen_prefix: Assertion `0' failed.
After:
66: Error: Object mapping data should be a value, not prefix
synproxy name ip saddr map { 192.168.1.0/24 : "v*" }
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Exercise payload transport match and mangle for inet, bridge and netdev
families with IPv4 and IPv6 packets.
To cover kernel patch ("netfilter: nf_tables: set transport offset from
mac header for netdev/egress").
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The reproducer crashes during concat evaluation, as the
exthdr expression lacks a datatype.
This should never happen, i->dtype must be set.
In this case the culprit is tcp option parsing, it will
wire up a non-existent template, because the "nop" option
has no length field (1 byte only).
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Alternative would be to refactor this and move this into the parsers
(bison, json) instead of this hidden re-parsing.
Fixes: 627c451b2351 ("src: allow variables in the chain priority specification")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The calculation of the dynamic on-stack array is incorrect,
the scratch space can be too low which gives stack corruption:
AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffdb454f064..
#1 0x7fabe92aaac4 in __mpz_export_data src/gmputil.c:108
#2 0x7fabe92d71b1 in netlink_export_pad src/netlink.c:251
#3 0x7fabe92d91d8 in netlink_gen_prefix src/netlink.c:476
div_round_up() cannot be used here, it fails to account for register
padding. A 16 bit prefix will need 2 registers (start, end -- 8 bytes
in total).
Remove the dynamic sizing and add an assertion in case upperlayer
ever passes invalid expr sizes down to us.
After this fix, the combination is rejected by the kernel
because of the maps' wrong data size, before the fix userspace
may crash before.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The kernel will reject this too, but unfortunately nft may try
to cram the data into the underlying libnftnl expr.
This causes heap corruption or
BUG: nld buffer overflow: want to copy 132, max 64
After:
Error: Concatenation of size 544 exceeds maximum size of 512
udp length . @th,0,512 . @th,512,512 { 47-63 . 0xe373135363130 . 0x33131303735353203 }
^^^^^^^^^
resp. same warning for an over-sized raw expression.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Else we get:
BUG: unknown expression type range
nft: src/netlink_linearize.c:909: netlink_gen_expr: Assertion `0' failed.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If prefix is used with a datatype that has less than 8 bits an
assertion is triggered:
src/netlink.c:243: netlink_gen_raw_data: Assertion `len > 0' failed.
This is esoteric, the alternative would be to restrict prefixes
to ipv4/ipv6 addresses.
Simpler fix is to use round_up instead of divide.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
This breaks existing behaviour, add a test case so this is caught in
the future.
The reverted test case will be brought back once a better fix
is available.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Before:
nft: gmputil.c:77: mpz_get_uint8: Assertion `cnt <= 1' failed.
After: Error: reject code must be integer in range 0-255
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
AddressSanitizer: heap-buffer-overflow on address 0x6020000003af ...
#0 0x7f9a83cbb402 in tchandle_type_parse src/meta.c:89
#1 0x7f9a83c6753f in symbol_parse src/datatype.c:138
strlen() - 1 can underflow if length was 0.
Simplify the function, there is no need to duplicate the string
while scanning it.
Expect the first strtol to stop at ':', scan for the minor number next.
The second scan is required to stop at '\0'.
Fixes: 6f2eb8548e0d ("src: meta priority support using tc classid")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
BUG: invalid range expression type symbol
nft: expression.c:1494: range_expr_value_high: Assertion `0' failed.
After:
range_expr_value_high_assert:5:20-27: Error: Could not resolve protocol name
elements = { 100-11.0.0.0, }
^^^^^^^^
range_expr_value_high_assert:7:6-7: Error: set definition has conflicting key (ipv4_addr vs inet_proto)
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
nat_concat_map() requires a datamap, else we crash:
set->data is dereferenced.
Also update expr_evaluate_map() so that EXPR_SET_REF is checked there
too.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
prio_spec may contain an embedded expression, release it.
We also need to release the device expr and the hook string.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
cmd_alloc() will free the chain, so we must close the scope opened
in chain_block_alloc beforehand.
The included test file will cause a use-after-free because nft attempts
to search for an identifier in a scope that has been freed:
AddressSanitizer: heap-use-after-free on address 0x618000000368 at pc 0x7f1cbc0e6959 bp 0x7ffd3ccb7850 sp 0x7ffd3ccb7840
#0 0x7f1cbc0e6958 in symbol_lookup src/rule.c:629
#1 0x7f1cbc0e66a1 in symbol_get src/rule.c:588
#2 0x7f1cbc120d67 in nft_parse src/parser_bison.y:4325
Fixes: a66b5ad9540d ("src: allow for updating devices on existing netdev chain")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
table inet filter {
ct helper sip-5060u {
type "sip" protocol udp
l3proto ip
}5060t {
type "sip" protocol tcp
l3pownerip
}
Will close the 'ct' scope twice, it has to be closed AFTER the separator
has been parsed.
While not strictly needed, also error out if the protocol is already
given, this provides a better error description.
Also make sure we release the string in all error branches.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
obj_free() won't release them because ->type is still 0 at this
point.
Init this to CT_TIMEOUT.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
| |
The assertion is too strict, as found by afl++:
typeof iifname . ip saddr . meta ipsec
elements = { "eth0" . 10.1.1.2 . 1 }
meta ipsec is boolean (1 bit), but datasize of 1 is set at 8 bit.
Fixes: 22b750aa6dc9 ("src: allow use of base integer types as set keys in concatenations")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
We must release the expression here, found via afl++ and
-fsanitize-address build.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Consider this:
counter_stmt : counter_stmt_alloc
| counter_stmt_alloc counter_args
counter_stmt_alloc : COUNTER { $$ = counter_stmt_alloc(&@$); }
| COUNTER NAME stmt_expr
{
$$ = objref_stmt_alloc(&@$);
$$->objref.type = NFT_OBJECT_COUNTER;
$$->objref.expr = $3;
}
;
counter_args : counter_arg { $<stmt>$ = $<stmt>0; }
| counter_args counter_arg
;
counter_arg : PACKETS NUM { $<stmt>0->counter.packets = $2; }
[..]
This has 'counter_stmt_alloc' EITHER return counter or objref statement.
Both are the same structure but with different (union'd) trailer content.
counter_stmt permits the 'packet' and 'byte' argument.
But the 'counter_arg' directive only works with a statement
coming from counter_stmt_alloc().
afl++ came up with following input:
table inet x {
chain y {
counter name ip saddr bytes 1.1.1. 1024
}
}
This clobbers $<stmt>->objref.expr pointer, we then crash when
calling expr_evaluate() on it.
Split the objref related statements into their own directive.
After this, the input will fail with:
"syntax error, unexpected bytes, expecting newline or semicolon".
Also split most of the other objref statements into their own blocks.
synproxy seems to have same problem, limit and quota appeared to be ok.
v1 added objref_stmt to stateful_stmt list, this is wrong, we will
assert when generating the 'counter' statement.
Place it in the normal statement list so netlink_gen_stmt_stateful_assert
throws the expected parser error.
Fixes: dccab4f646b4 ("parser_bison: consolidate stmt_expr rule")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
The includes test files cause:
BUG: chain is too large (257, 256 max)nft: netlink.c:418: netlink_gen_chain: Assertion `0' failed.
Error out in evaluation step instead.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Fix this warning due to missing coverage:
tests/py/any/meta.t.json.got: WARNING: line 2: Wrote JSON equivalent for rule meta mark set vlan id map { 1 : 0x00000001, 4095 : 0x00004095 }
ERROR: did not find JSON equivalent for rule 'meta mark set vlan id map @map1
Fixes: 8d3de823b622 ("evaluate: reset statement length context before evaluating statement")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch consolidates ctx->stmt_len reset in stmt_evaluate() to avoid
this problem. Note that stmt_evaluate_meta() and stmt_evaluate_ct()
already reset it after the statement evaluation.
Moreover, statement dependency can be generated while evaluating a meta
and ct statement. Payload statement dependency already manually stashes
this before calling stmt_evaluate(). Add a new stmt_dependency_evaluate()
function to stash statement length context when evaluating a new statement
dependency and use it for all of the existing statement dependencies.
Florian also says:
'meta mark set vlan id map { 1 : 0x00000001, 4095 : 0x00004095 }' will
crash. Reason is that the l2 dependency generated here is errounously
expanded to a 32bit-one, so the evaluation path won't recognize this
as a L2 dependency. Therefore, pctx->stacked_ll_count is 0 and
__expr_evaluate_payload() crashes with a null deref when
dereferencing pctx->stacked_ll[0].
nft-test.py gains a fugly hack to tolerate '!map typeof vlan id : meta mark'.
For more generic support we should find something more acceptable, e.g.
!map typeof( everything here is a key or data ) timeout ...
tests/py update and assert(pctx->stacked_ll_count) by Florian Westphal.
Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
Add missing json output.
Fixes: 99ab1b8feb16 ("rule: never merge across non-expression statements")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
Thanks to autocomplete i didn't notice this earlier,
make this lowercase.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
tcp option 254 length ge 4
... will segfault.
The crash bug is that tcpopt_expr_alloc() can return NULL if we cannot
find a suitable template for the requested kind + field combination,
so add the needed error handling in the bison parser.
However, we can handle this. NOP and EOL have templates, all other
options (known or unknown) must also have a length field.
So also add a fallback template to handle both kind and length, even
if only a numeric option is given that nft doesn't recognize.
Don't bother with output, above will be printed via raw syntax, i.e.
tcp option @254,8,8 >= 4.
Fixes: 24d8da308342 ("tcpopt: allow to check for presence of any tcp option")
Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
tests/shell/testcases/bogons/nft-f/set_definition_with_no_key_assert
BUG: unhandled key type 2
nft: src/intervals.c:59: setelem_expr_to_range: Assertion `0' failed.
[ This bug doesn't trigger anymore due to
1949a63215b4 ("evaluate: reject set definition with no key") ]
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
monitor is missing concatenated set ranges support.
Fixes: 8ac2f3b2fca3 ("src: Add support for concatenated set ranges")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
feature probe script leaves a ruleset in place, flush it once probing is
complete.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
We release ->dtype twice, will either segfault or assert
on dtype->refcount != 0 check in datatype_free().
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
mapping_With_invalid_datatype_crash:1:8-65: Error: Implicit map expression without known datatype
bla to tcp dport map { 80 : 1.1.1.1 . 8001, 81 : 2.2.2.2 . 9001 } bla
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
This will crash as set->data is NULL, so check that SET_REF is pointing
to a map:
Error: candidates_ipv4 is not a map
tcp dport 10003 ip saddr . tcp dport @candidates_ipv4 add @candidates_ipv4 { ip saddr . 10 :0004 timeout 1s }
~~~~~~~~~~~~~~~~
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
... this will cause an assertion in netlink linearization, catch this
at eval stage instead.
before:
BUG: unknown expression type range
nft: netlink_linearize.c:908: netlink_gen_expr: Assertion `0' failed.
after:
/unknown_expr_type_range_assert:3:31-40: Error: Meta expression cannot be a range
meta mark set 0x001-3434
^^^^^^^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
prefer
binop_with_different_basetype_assert:3:29-35: Error: Binary operation (<<) with different base types (string vs integer) is not supported
oifname set ip9dscp << 26 | 0x10
^^^^^^^~~~~~~
to assertion failure.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
| |
i->dtype->basetype can be NULL.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
BUG: invalid mapping expression binop
nft: src/evaluate.c:2027: expr_evaluate_map: Assertion `0' failed.
After:
tests/shell/testcases/bogons/nft-f/invalid_mapping_expr_binop_assert:1:22-25: Error: invalid mapping expression binop
xy mame ip saddr map h& p p
~~~~~~~~ ^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
large '& VAL' results in:
src/evaluate.c:531: expr_evaluate_bits: Assertion `masklen <= NFT_REG_SIZE * BITS_PER_BYTE' failed.
Turn this into expr_error().
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The command `nft [-j] list ruleset | nft [-j] --check -f -` should never
fail. "test-wrapper.sh" already checks for that.
However, previously, we would run check against the .nft/.json-nft
files. In most cases, the generated ruleset and the files in git are
identical. However, when they are not, we (also) want to run the check
against the generated one.
This means, we can also run this check every time, regardless whether a
.nft/.json-nft file exists.
If the .nft/.json-nft file is different from the generated one, (because
a test was skipped or because there is a bug), then also check those
files. But this time, any output is ignored as failures are expected
to happen. We still run the check, to get additional coverage for
valgrind or santizers.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
Error out instead of 'nft: gmputil.c:67: mpz_get_uint32: Assertion `cnt <= 1' failed.'.
Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
We don't want a dump file here, the test has elements with
timeouts, listing will differ depending on timing ("expires $random seconds").
Fixes: 4890211e188a ("tests: shell: add test case for catchall gc bug")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
nft --check -f tests/shell/testcases/bogons/nft-f/set_without_key
Segmentation fault (core dumped)
Fixes: 56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Check for bug fixed with kernel
commit 93995bf4af2c ("netfilter: nf_tables: remove catchall element in GC sync path").
Reported-by: lonial con <kongln9170@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
|