Commit Graph

12951 Commits (3b13008c1b6b994df0ae3f702c24780fd253ec32)
 

Author SHA1 Message Date
Victor Julien 259bd8aa92 detect/address: validate netmasks
Only accept netmask in dotted quad notation if they can be turned
into a CIDR.

According to rfc 4632, CIDR (compat) netmasks are all that should be
used.

Bug: #5168.
3 years ago
Victor Julien 4020e2faa7 detect/iponly: break out range insert code
So we can reuse it.
3 years ago
Victor Julien a67b97e14c util/cidr: add util to convert netmask to cidr 3 years ago
Philippe Antoine eb189e805a src: use u8_tolower everywhere
Ticket: 4516

Instead of basic to_lower to get the cast to avoid warning
about integer

Sames goes for u8_toupper
3 years ago
Philippe Antoine 3fd8e908f8 range: better closing for out of order ranges
Ticket: 5132

In case of a duplicate range, we can return early, because
there is no new data to process.

More importantly, this commit adds a check about wether the file
got closed meanwhile, so that this just completed out of order
range, even if it brings new data, is now irrelevant.
This can happen for instance if there was a gap...
3 years ago
Philippe Antoine bfcd6cb46a range: validity check when end is bigger than size
Ticket: 5132

Down the line, HttpRangeOpenFileAux assumes the range has a
valid value when doing buflen = end - start + 1;
3 years ago
Modupe Falodun 14b21de306 detect-dnp3: remove dnp3_data unittests
These tests are reimplemented in Suricata-Verify

Task: 4911
3 years ago
Philippe Antoine ae6c416972 util/mime: fix integer warnings
Ticket: 4516
3 years ago
Victor Julien ec01a94a5f detect: minor debug fixup 3 years ago
Victor Julien b7526bf4e6 decode/vntag: don't leak memory in tests 3 years ago
Victor Julien 0437ca61ff unittests: clean up packet clear logic 3 years ago
Victor Julien f07d5b2d89 decode: release refs from PacketFree
Mostly helps unittests.
3 years ago
Victor Julien 49a36bb323 detect/iprep: fix host locking issues
Separate the code paths between reusing a Packet stored host reference
and fetching a new reference from the host hash.

This addresses the issue where in some conditions use_cnt could get
desync'd.

Bug: #2802.
3 years ago
Victor Julien 172d2b28a5 iprep: unify free handling
Introduce a new util function to free a Hosts iprep code. It also
handles the Host use_cnt decrement.

This change makes sure we also decrement the use_cnt when cleaning
up when shutting down the host table.

Move the BUG_ON check for use_cnt into the HostClearMemory() func
to check it in more cases.
3 years ago
Philippe Antoine a6a6f6d538 bytejump: fix ubsan warning
Instead of checking the offset, we checked the pointer after
adding the offset ot it...
3 years ago
Jeff Lucovsky 4f2f745bed detect/ipproto: Use builtin protocol table
Issue 5072

This commit causes the built-in protocol table to be used for protocol
name and number validation.
3 years ago
Jeff Lucovsky 3bd1d258a9 detect/tests: Register protoname tests
Issue: 5072

This commit registers the proto-name unit tests.
3 years ago
Jeff Lucovsky b524967257 detect/ipproto: Add init/release functions
Issue: 5072

This commit insures that the protocol name hashtables are initialized
and released.
3 years ago
Jeff Lucovsky ff0cf89738 util/proto: Protocol-name functions
Issue: 5072

This commit adds utility functions handling protocol names.
3 years ago
Jeff Lucovsky 1e2883602b error/hash: Add error code for hash add failures
Issue: 5072
3 years ago
Andreas Dolp d4144c04cd Doc: Fix typo in documentation of suricata.yaml. 3 years ago
Juliana Fajardini d38e294a43 build-info: add info about fuzztargets
We were missing that information from the Development information
3 years ago
Shivani Bhardwaj 015c9fe1e3 doc: add usage of flowbits OR op
Ticket 5130
3 years ago
Juliana Fajardini 6c616d84b1 devguide: clarify style guide for getframe funcs
As the GetFrameIdByName can be probed, we must warn developers not to
leave any output in them, or misleading messages could be printed.

Task #5129
3 years ago
Jeff Lucovsky 6232c94235 threads: Honor per-thread stack size setting
Issue: 4550

This commit adjusts the per-thread stack size if a size has been
configured. If the setting has not been configured, the default
per-thread stack size provided by the runtime mechanisms are used.
3 years ago
Jeff Lucovsky e4d60f451b config/thread: Use config'd per-thread stack size
Issue: 4550

This commit checks if there's a config setting for threading.stack-size
and assigns the value to a global variable for use during thread
creation.
3 years ago
Jeff Lucovsky d79a317cea suricata.yaml: Add per-thread stack size setting
Issue: 4550
3 years ago
Jeff Lucovsky deb49862cd config/debug: Debug probe for getattr_np
This commit adds a probe for a non-portable function to be used in
diagnostic debug display of a thread's stack size.
3 years ago
Jeff Lucovsky 117e11b0ae doc: Describe per-thread stack size config setting
Issue: 4550

This commit documents the new per-thread stack-size setting. Some
systems have a small default value that is not suitable for Suricata's
multi-threaded architecture and adjustment may be required.
3 years ago
Victor Julien 07b1100713 nfs: clean up partial record handling
There should be no remaining data after parsing the partial
RPC record, so don't handle it but instead add a debug validation
bug on.

Successful processing for NFSv3 read/write records returns
AppLayerResult::ok() directly as all data is consumed.
3 years ago
Victor Julien d85b77cad0 nfs3: improve read validation; fix partial handling 3 years ago
Victor Julien 4418fc1b02 nfs3: fix partial write record handling 3 years ago
Victor Julien 5baf94e40d nfs3: enforce more values
Enforce values of a number of u32's that are used as bools or for
really low values.
3 years ago
Victor Julien 1c57e3c18d rpc: enforce various field values
Minimal frag_len. Correct msgtype and others.
3 years ago
Victor Julien 64d8a1e16e nfs/rpc: update full record parsers to be more exact
Instead of 'take'ing all data for the RPC prog_data and then
letting the higher level parsers figure out which part to use
take the exact amount.
3 years ago
Victor Julien bfb5ae867e nfs: break out partial record handling 3 years ago
Victor Julien fe76ab1803 nfs/rpc: enforce length field limits
Limits based on the Linux kernel limits. Then multiplied a few times
to allow for other implementations to have higher limits.
3 years ago
Victor Julien 5ecb626e50 nfs4: verify bool fields 3 years ago
Victor Julien a0c0471f1f output: fix timestamp missing usecs
On ARM 32bit with Musl `tv_usecs` is defined as `int64_t` which lead to
CreateIsoTimeString() printing all zeros on the usecs. Work around this
by first assigning to a `int64_t` and then updating the expected format
string to accept `int64_t`.

Bug: #5094.
3 years ago
Jason Ish b1c09369af rust/derive: pin proc-macro-crate to v1.1.0.
The just released proc-macro-crate v1.1.2 requires at least Rust 1.53.
Pin to the previous release for now.
3 years ago
Pierre Chifflier b8f767d84c rust/mime: convert parser to nom7 3 years ago
Victor Julien 8a73b242e3 detect/address: use common cidr code 3 years ago
Victor Julien 38aec1439c radix: fix unittests after stict checks 3 years ago
Victor Julien 7fd6fe732b radix: improve address range handling
Handle non-exact address ranges from string. This can come directly
from user input, so here it is accepted but the address is converted
to the address range start. A warning will be issued.

Debug validation checks are added to catch this.

This issue could lead to bad input from iprep (with cidr), defrag config
and htp server personalities to produce a bad radix tree.

Bug: #5084.
Bug: #5085.
Bug: #5086.
3 years ago
Victor Julien 51d4e0dced detect/iponly: fix netmask handling
If the ipaddress was not the address range start, it was not masked to turn
it into that. So 1.2.3.4/24 was not stored as address 1.2.3.0 with netmask 24,
but as 1.2.3.4 with netmask 24. This was then propagated into the radix tree,
where it was used as an exact key in exact lookups, giving unexpected results.

This patch implements the netmask handling for IPv4 and IPv6, and adds a set
of tests for it.

Bug: #5081.
Bug: #5066.
3 years ago
Victor Julien 311085dd34 radix: fix unittest not cleaning up 3 years ago
Victor Julien 860daceb04 detect/iponly: update SigNumArray comment 3 years ago
Victor Julien d04dface20 radix: cleanup test 3 years ago
Victor Julien 89b7ac0a60 radix: add tests for Bug #5066
Bug: #5066.
3 years ago
Victor Julien 6aa6e3f953 radix: fix FP/FN issue in IP-only
A bug was reported about the IP-only rules not correctly matching. This was
traced to the rules in question not getting recorded into the IP-only radix
tree correctly.

Sequence:

- 100.117.241.0/25 inserted into the tree

- 100.117.241.0/26 inserted into the tree

Both are part of the same radix node, but recorded by their different netmasks
in the user data portion.

Then faulty insert happens:

- 100.117.241.64/26

For reference, these net blocks compute to:

- 100.117.241.0/25:  100.117.241.0  - 100.117.241.127
- 100.117.241.0/26:  100.117.241.0  - 100.117.241.63
- 100.117.241.64/26: 100.117.241.64 - 100.117.241.127

The IP-only engine first does a search to get to the user data it may need to
include. It does so for with `SCRadixFindKeyIPV4ExactMatch` for single IPs, or
using `SCRadixFindKeyIPV4Netblock` in case of a netblock. Any "match" from
either of these is considered an "exact match" by the IP-only setup code.

This exact match expectation turned out to be wrong and
`SCRadixFindKeyIPV4Netblock` behaved more like "best match" instead, which is
a non-exact match, but its the next best match if no exact match is found.

The way the look up for 100.117.241.64/26 went wrong, is that it returned
the user data for 100.117.241.0/26. This happens as follows:

- first it would do an exact find, which didn't give a result

- then it removed bits from the keystream until it found a matching node
  and explore if any of the netmasks it contained matched. Here the first
  step of the bug started:

  it considered the netmask (with user data) a match that matched the
  number of bits of the matching key, but not of the actual range netmask cidr
  value.

  So in this case the number of shared bits between `100.117.241.0/25` and
  `100.117.241.64/26` was 25, so it assumed that the user data for the
  netmask 25 was the match.

  To summarize this step, there are 2 problems with this:
  1. it returns a match on something that isn't an exact match
  2. it considered the wrong netmask value

- the radix code then took the returned node, and did the netmask check
  again. This time it did use its own netmask value, so this time
  it did find the netmask 26 (+ user data). However because of the node that
  was returned, this netmask (+user data) belongs to `100.117.241.0`, not to
  `100.117.241.64`.

- the IP-only detection code was satisfied with what it assumed to be
  "exact match" and just updated the user data to include the user data that
  should have been associated with `100.117.241.64/26` to `100.117.241.0/26`.

This patch addresses the issue as follows:

It makes `SCRadixFindKeyIPV4Netblock` also return an exact match by propagating
the netmask in the search and in the evaluation of the stored netmasks.

It does away with the secondary netmask (+user data) evaluation.
`SCRadixFindKeyIPV4Netblock` is expected to handle this correctly.

The IP-only engine will fall back to the "not found" path, which does an explicit
"best match" lookup and then insert a new entry into the radix tree based on
the user data of the "best match".

Issue was present for IPv6 as well.

Bug: #5066.
3 years ago