| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
| |
Since fw->arp.arhln is of type __u8, its value will never become less
than zero.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Move the function to xshared.c for common use between legacy and xtables
sources. While being at it, silence a covscan warning triggered by that
function as it couldn't verify input buffers won't exceed IFNAMSIZ.
Therefore use snprintf() when writing to the local buffer.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These functions contain code which occurs in legacy's print_firewall()
functions, so use them there.
Rename them to at least make clear they print more than a single
address.
Also introduce ipv{4,6}_addr_to_string() which take care of converting
an address/netmask pair into string representation in a way which
doesn't upset covscan (since that didn't detect that 'buf' may not be
exceeded by the strings written into it.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
As covscan stated: "Trapping signals by number is not well defined.
Prefer signal names."
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Not a real problem here, but covscan got confused by one string
containing 'then' keyword.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
This eliminates the deprecation warning when compiling the sources.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
If xtables_strtoul() fails, it returns false and data pointed to by
parameter 'value' is undefined. Hence avoid copying that data in
xtables_strtoui() if the call failed.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Both affected functions check if 'oldopts' is NULL once but later seem
to ignore that possibility. To catch up on that, increment the pointer
only if it isn't NULL, also don't copy its content into the merged
options buffer in that case.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
This change originated from covscan complaining about the strcpy() call
with an unknown size source buffer. But in fact, the size is known (and
equal to the destination size), so pass a pointer to STRUCT_GETINFO to
alloc_handle() instead of it's fields separately. Hopefully this will
silence covscan.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
The variable is not read before being assigned the return value of
strtoul(), thefore the initialization is useless. And since after this
change parameter 'end' becomes unused, drop it as well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
Just like with libxt_conntrack, get rid of the temporary buffer. The
comment even states that it was copied from there, so just make them
identical again.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
In print_addr(), a resolved hostname is written into a buffer without
size check. Since BUFSIZ is typically 8192 bytes, this shouldn't be an
issue, though covscan complained about it. Fix the code by using
conntrack_dump_addr() as an example.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Since sinfo->flags is only 8 bytes large, checking for
XT_CONNTRACK_DIRECTION bit (which has value 1 << 12) will always return
false, so drop this dead code.
Fixes: c7fc1dae1e8f8 ("libxt_conntrack: dump ctdir")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Instead of using strcat() and assuming the name will fit, print into the
buffer using snprintf() which truncates the string as needed.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
This is a leftover from previous cleanup.
Fixes: 098ee2e91756c ("xtables-save: Ignore uninteresting tables")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
When reading sufficiently malformed input, parser might hit end of
loop without having written the current table name into curtable and
therefore calling strcmp() with uninitialized buffer. Avoid this by
setting curtable to zero upon declaration.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Typical covscan complaint, non-empty fall throughs should be marked as
such. There was but a single case which should break instead, namely in
libebt_log.c: It is not critical, since the next case merely asserts
'invert' being zero (which can't be as it was checked before). But while
being at it, introduce log_chk_inv() to consolidate the semantically
equal cases for the various log types.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This moves getethertype.c into libxtables so that both extensions and
xtables-nft-multi may use the implementations therein. New users are
libebt_arp and libebt_vlan which drop their own duplicated
implementations of getethertypebyname() for the shared one.
This change originated from a covscan report of extensions'
implementations not checking fopen() return value which should be
implicitly fixed by this as well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
The code obviously tries to assert that nft_table_builtin_find()
returned a valid pointer before dereferencing it, but the wrong argument
was given. Assume this is just a typo and insert the missing underscore.
Fixes: 9b896224e0bfc ("xtables: rework rule cache logic")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Although it remains unclear what the original intention behind the
affected code was, but 'tmp + 1' always evaluates true since 'tmp' is a
pointer value.
Cc: Evgeniy Polyakov <johnpol@2ka.mxt.ru>
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Commit 56d7ab42f3782 ("libxt_string: Avoid potential array out of bounds
access") tried to fix parse_hex_string() for overlong strings but the
change still allowed for 'sindex' to become XT_STRING_MAX_PATTERN_SIZE
which leads to access of first byte after info->pattern. This is not
really a problem because it merely overwrites info->patlen before
calling xtables_error() later, but covscan still detects it so it's
still worth fixing.
The crucial bit here is that 'sindex' has to be incremented at end of
the last iteration since its value is used for info->patlen. Hence just
move the overflow check to the beginning of the loop.
Fixes: 56d7ab42f3782 ("libxt_string: Avoid potential array out of bounds access")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When running iptables-nft-save with other tables present, the dump
succeeded but the tool complained about those other tables. In an
environment where iptables-nft and nftables are uses in parallel, this
is an expected situation, so only complain about incompatible builtin
tables.
While being at it, move the table existence check from __do_output()
into do_output() since the former may be called from
nft_for_each_table() in which case the table is guaranteed to exist.
Also use nft_table_builtin_find() in nft_is_table_compatible() instead
of open-coding the search by name in h->tables.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
| |
Just like revision v1, but cgroup path field is smaller.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Initial motivation for this was a covscan report for potential array out
of bounds access in REJECT_xlate (a false-positive, because all possible
values of reject->with occur in reject_table_xlate).
Use reject types as array indices of reject_table so that reject->with
serves as array index. Also merge reject_table_xlate into it.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
The pattern index variable 'sindex' is bounds checked before
incrementing it, which means in the next loop iteration it might already
match the bounds check condition but is used anyway.
Fix this by incrementing the index before performing the bounds check.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Fix the parameter check in nft_ebt_standard_target() to avoid an array
out of bounds access in ebt_standard_targets.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These calls to memset() are passed a length argument which exceeds
t->target.u.user.name's length by one byte and hence overwrite
t->target.u.user.revision as well (relying upon no padding to happen
between both).
Avoid this obscure behaviour by passing the correct field size and
explicitly overwriting 'revision' field.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
If entry->type is to be used as array index, it needs to be at max one
less than that array's size.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
Passing --wait option to iptables-nft-restore led to program abort
because the flag parameter was not skipped. Mimick iptables-restore
behaviour when encountering --wait or --wait-interval options (but still
ignore the parameter).
Fixes: b9d7b49d84bc2 ("xtables-compat: restore: sync options with iptables-restore")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Commit f8e29a13fed8d ("xtables: avoid bogus 'is incompatible' warning")
fixed for compatibility checking to extend over all chains, not just the
relevant ones. This patch does the same for rules: Make sure only rules
belonging to the relevant table are being considered.
Note that comparing the rule's table name is sufficient here since the
table family is already considered when populating the rule cache.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Make it more clear where the instruction ends, and where
what it does begins.
Signed-off-by: Joseph C. Sible <josephcsible@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The logic to replicate 'iptables-restore --noflush' behaviour of
flushing custom chains if listed in the dump was broken for chains being
referenced. A minimal dump reproducing the issue is:
| *filter
| :foobar - [0:0]
| -I INPUT -j foobar
| -A foobar -j ACCEPT
| COMMIT
With --noflush, this can be restored just once in iptables-nft-restore.
Consecutive attempts return an error since xtables tries to delete the
referenced chain and recreate it instead of performing a real flush.
Fix this by really flushing the custom chain in 'chain_user_flush'
callback and running 'chain_user_add' callback only if the chain doesn't
exist already.
Fixes: df3d92bec6007 ("xtables-compat-restore: flush user-defined chains with -n")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Setting this bit in cs->fw6.ipv6.flags was done only for rules parsed
from command line, not for those read from kernel. As a result,
appropriate rules could not be deleted. A simple test case is:
| # ip6tables-nft -A INPUT -p tcp -j ACCEPT
| # ip6tables-nft -D INPUT -p tcp -j ACCEPT
| iptables: Bad rule (does a matching rule exist in that chain?).
Since the flag is not used anywhere in xtables-nft, dropping its use fixes
the bug as well as setting it in both cases.
Fixes: 5ee03e6df4172 ("xtables: Use meta l4proto for -p match")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Make sure return codes match legacy ones at least for a few selected
commands typically used to check ruleset state.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Comment match allocation in command_match() and
nft_rule_to_iptables_command_state() were misaligned in that the latter
set match_size to just what is required instead of what the match needs
at maximum like the further. This led to failure when comparing them
later and therefore a rule with a comment could not be deleted.
For comments of a specific length, the udata buffer is padded by
libnftnl so nftnl_rule_get_data() returns a length value which is larger
than the string (including NULL-byte). The trailing data is supposed to
be ignored, but compare_matches() can't not know about that detail and
therefore returns a false-negative if trailing data contains junk. To
overcome this, use strncpy() when populating match data in
nft_rule_to_iptables_command_state(). While being at it, make sure
comment match allocation in that function is identical to what
command_match() does with regards to data allocation size. Also use
xtables_calloc() which does the required error checking.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
Layer 4 protocol name "mobility-header" is not known by nft, so it's
neither printed nor accepted on input. Hence fix the test instead of
code.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
The xlate function sharing here does not quite work since in
ebtables-translate, extensions are supposed to append whitespace. Fix
this by introducing a simple wrapper.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
| |
These fix reports for definitely lost blocks in valgrind. Not really
memleaks, but due to nft_handle going out of scope they're counted as
lost. Still worth fixing though since it reduces noise when auditing
code for real issues.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is a partial revert of my previous commit with similar subject - it
missed to apply the needed changes to ebtables-translate as well and on
top of that still left some leaks and use-after-frees in place. The new
strategy is to make ebtables extension loading compatible with that of
xtables, because otherwise the heavy code sharing between
ebtables-translate and iptables-translate will cause trouble.
Basically, ebt_add_match() and ebt_add_watcher() copy what xtables'
command_match() does, but after the actual extension argument parsing
has already happened. Therefore they duplicate the loaded match along
with its data and reset the original one to default state for being
reused (e.g., by ebtables-restore). Since mflags/tflags are cleared
while doing so, clearing them for all loaded extensions in
do_commandeb() is not necessary anymore.
In ebt_command_default() (where extension parameter parsing happens),
the list of added extensions to the current rule are consolidated first
so no duplicate extension loading happens.
With the above in place, ebt_cs_clean() can be reverted to its old
state.
Apart from sharing command_jump() function with ebtables-translate, make
use of nft_init_eb() there, as well.
Fixes: aa7fb04fcf72c ("ebtables: Review match/target lookup")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use of this static variable causes trouble as it affects all instances
of this target. So calling xs_init_target() for one instance invalidates
all the others.
Moving the variable into target private data seems not possible since
that would change the target's size and therefore it wouldn't match
anymore with what kernel expects.
So just get rid of it entirely. If a user "forgets" to set a mark value,
the default value of zero applies.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
Mostly to reduce noise from valgrind output, add missing calls to
destroy iterators in nft.c and add cleanup for the populated nft_handle
in xtables_eb_save_main().
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previous review of match/target lookup did not consider
xtables-eb-translate.c which contains the same code. Fix parsing of
target/match arguments there as well by introducing
ebt_command_default() which consolidates the previously duplicated code.
One notable quirk in comparison to the similar xtables code: Since
ebtables allows for negations in ugly places (e.g. '--arp-opcode ! 1'),
ebt_check_inverse2() has to be called first.
Fixes: aa7fb04fcf72c ("ebtables: Review match/target lookup")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
| |
This nit was discovered when comparing do_commandeb() with
do_commandeb_xlate(): Since 'cs' is initialized upon declaration
already, initialization of field '.eb.bitmask' may be moved there as
well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
Aparently, this is a leftover from the compat->nft naming change in
created binary (symlinks).
Fixes: be70918eab26e ("xtables: rename xt-multi binaries to -nft, -legacy")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
With audit logging being supported by nftables as a simple (fake) log
level, translating AUDIT target is easy. Especially since xt_AUDIT in
kernel doesn't quite care about --type parameter.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use of payload expression to match against IPv6 nexthdr field does not
work if extension headers are present. A simple example for that is
matching for fragmented icmpv6 traffic. Instead, generate a 'meta
l4proto' expression which works even if extension headers are present.
For consistency, apply the same change to iptables-nft as well.
No adjustment to reverse path required as the needed bits were added by
commit 6ea7579e6fe24 ("nft: decode meta l4proto") already.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch fixes the crash when registering the hashlimit extension
with xtables during init_extensions(when built with static libs) .
The option validation function xtables_option_metavalidate has a
loop termination condition of the entry name being NULL. The loop
does not terminate when validating hashlimit_mt_opts_v2 which causes
a crash on derefencing an invalid entry.
Signed-off-by: Heena Sirwani <heenasirwani@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Trying to set a chain's policy in an invalid table resulted in a
segfault. Reproducer was:
| # iptables -t broute -P BROUTING ACCEPT
Fix this by aborting in nft_chain_new() if nft_table_builtin_find()
returned NULL for the given table name.
For an illustrative error message, set errno to ENXIO in the above case
and add an appropriate Mesage to nft_strerror().
While being at it, improve the error message if an invalid policy was
given. Before:
| # iptables-nft -t filter -P INPUT ACCEPTdf
| iptables: Incompatible with this kernel.
After:
| # iptables-nft -t filter -P INPUT ACCEPTdf
| iptables: Bad policy name. Run `dmesg' for more information.
Third unrelated change in this patch: Drop error checking of
nft_chain_set() in do_commandx(): The function never returns negative,
so that check never yielded true.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The previous fix for reference counts in iptables-nft output wasn't
complete: While iptables lists the number of references for each custom
chain (i.e., the number of jumps to it), ebtables lists number of
entries (i.e., the number of rules contained) for each chain. Both used
the same value for it, although they are different metrics.
Fix this by passing both numbers separately to the 'print_header'
callback so that each tool may print the desired value.
Fixes: a0698de9866d2 ("xtables: Do not count rules as chain references")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
To be consistent with legacy iptables, calling -S with a non-existing
chain should lead to an error message. This is how some scripts find out
whether a user-defined chain exists or not.
Make sure doing the same for an existing chain does succeed, even if an
invalid rule number was given.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|