Commit Graph

10614 Commits (7e2ed11a11cae89bc07e9a25425192581687993d)

Author SHA1 Message Date
Juliana Fajardini a6bda3596b unittests: alloc Packet with PacketGetFromAlloc
Some unittests used SCMalloc for allocating new Packet the unittests.
While this is valid, it leads to segmentation faults when we move to
dynamic allocation of the maximum alerts allowed to be triggered by a
single packet.

This massive patch uses PacketGetFromAlloc, which initializes a Packet
in such a way that any dynamic allocated structures within will also be
initialized.

Related to
Task #4207
4 years ago
Shivani Bhardwaj 6d2a2a0731 detect/dataset: fix space condition in rule lang
If there is a space following a keyword that does not expect a value,
the rule fails to load due to improper value evaluation.
e.g. Space after "set" command
alert http any any -> any any (http.user_agent; dataset:set  ,ua-seen,type string,save datasets.csv; sid:1;)

gives error
[ERRCODE: SC_ERR_UNKNOWN_VALUE(129)] - dataset action "" is not supported.

Fix this by handling values correctly for such cases.
4 years ago
Shivani Bhardwaj 7366396011 detect/dataset: cleanup dead code 4 years ago
Victor Julien 2b5eeab7d4 detect/urilen: don't pass null pointer to pcre2 free
Bug #5228.
4 years ago
Victor Julien 087151ddc3 detect/mpm: initialization micro optimization 4 years ago
Victor Julien 54a6dd09dd detect: pattern id assignment through hash table
Only consider active part of the pattern for mpm (so consider chop).

Move data structure to hash list table over the custom array logic.
4 years ago
Victor Julien a14854bce9 detect: keyword list to hash to improve perf
Since the switch to pcre2 this was much more heavily used, which
would lead to measurable time spent in list handling.
4 years ago
Victor Julien 9e6370ae2e detect: optimize mpm-engine setup
Instead of a loop over the rules in a group *per engine* do a single
loop in which all the engines are prepared in parallel.
4 years ago
Victor Julien 3352c0bee4 detect: initialization optimization
A lot of time was spent in `SigMatchListSMBelongsTo` for the `mpm_sm`.

Optimize this by keeping the value at hand during Signature parsing and
detection engine setup.
4 years ago
Victor Julien b804a84c93 hash: constify data input 4 years ago
Victor Julien 4b0e3d79bb detect/analyzer: support frames in pattern dump 4 years ago
Victor Julien 47629b7aeb detect/filemagic: don't pass unused pointer 4 years ago
Arne Welzel 8ef066318d flow-manager: fix off-by-one in flow_hash row allocation
The current code doesn't cover all rows when more than one flow manager is
used. It leaves a single row between ftd->max and ftd->min of the next
manager orphaned. As an example:

    hash_size=1000
    flowmgr_number=3
    range=333

    instance  ftd->min  ftd->max
    0         0         333
    1         334       666
    2         667       1000

    Rows not covered: 333, 666
4 years ago
Victor Julien 9537d119b9 http: fix reassembled range file accounting 4 years ago
Victor Julien 6d30f4442c http2: fix file accounting for ranged files
Increment files_opened for tx that 'gets' reassembled ranged file
4 years ago
Victor Julien 54d34c96f5 files: open/log debug validation bugon
Meant to find more cases where there is a mismatch.
4 years ago
Philippe Antoine cfcade58ad http: move xff logging to alert object
Ticket: 4860

instead of root field
4 years ago
Philippe Antoine 862e84877f ssl: first pass limit when allocating buffer for certificates
With this check, on the first packet of a certificate presenting
a length of 16Mbytes, we only allocate up to 65Kb

When we get to the point where need more than 65Kb, we realloc
to the true size.

With this check, it makes it more expensive for an attacket to use
this allocation as a way to trigger ressource exhaustion...
4 years ago
Philippe Antoine 99b3443369 smtp: check if there is a transaction to close
Ticket: 4948

When parsing the response for starttls
4 years ago
Philippe Antoine 68fa080069 tmqh: fix possible null dereference
Coverity ID: 1502953

As we check just on the next line my_pool against NULL, we should
not dereference it, even for debug validation
4 years ago
Philippe Antoine 568ab841d8 detect: remove dead code about xbits keyword 4 years ago
Philippe Antoine 4640b15d8c log: prevents use of uninitialized variable
Even if the code seems unreachable for now
4 years ago
Jason Ish 8d1e4a1d0b detect-content: error on single char hex pairs
Fix parsing of content like "|aa b cc|" which was parsed as "|aa bc|"
without error or warning. This will now fail out, requiring all hex
values to be 2 chars.

Ticket #5201
4 years ago
Victor Julien 6e90bf4739 streaming: remove unused 'auto slide' support
Add debug validation checks for "impossible" conditions.
4 years ago
Philippe Antoine 00da0d3420 detect: makes config keyword really require a flow
Ticket: 4972

Completes commit c3a220647

DETECT_CONFIG is added as DETECT_SM_LIST_POSTMATCH and not
as DETECT_SM_LIST_MATCH as other keywords handled in SignatureCreateMask
4 years ago
Modupe Falodun 54bc43d3ed detect-pcre: remove unittests
These tests are reimplemented in Suricata-Verify

Task: 4911
4 years ago
Philippe Antoine df83f7899d fuzz: fix integer warnings
Ticket: 4516
4 years ago
Philippe Antoine 5790280c95 utils: fix integer warnings in r files
Ticket: 4516
4 years ago
Philippe Antoine dca76a45a8 stream-tcp: fix integer warnings
Ticket: 4516
4 years ago
Philippe Antoine 068fb700df util: fix int warnings in tm threads
Ticket: 4516
4 years ago
Philippe Antoine 1bb51d114c util: fix int warnings in unit tests
Ticket: 4516
4 years ago
Philippe Antoine b3ab126394 util: fix int warnings
Ticket: 4516
4 years ago
Philippe Antoine c3a220647b detect: only apply ConfigApplyTx with app-layers
Ticket: 4972

Otherwise, it makes no sense to look for a tx...
4 years ago
Juliana Fajardini e5838b8193 applayer/frame: remove output from GetFrame funcs
As these functions can be probed, having output there results in
misleading output.
4 years ago
Modupe Falodun 44208010db detect-dce-iface: remove unittests
These tests are reimplemented in Suricata Verify

Task: 4911
4 years ago
Victor Julien 935ea745f5 detect/iponly: add tests for 5168 4 years ago
Victor Julien 053b2b3b5b detect/address: minor unittest cleanup 4 years ago
Victor Julien 79b7b7a0dd detect/iponly: validate netmask
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.
4 years ago
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.
4 years ago
Victor Julien 4020e2faa7 detect/iponly: break out range insert code
So we can reuse it.
4 years ago
Victor Julien a67b97e14c util/cidr: add util to convert netmask to cidr 4 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
4 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...
4 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;
4 years ago
Modupe Falodun 14b21de306 detect-dnp3: remove dnp3_data unittests
These tests are reimplemented in Suricata-Verify

Task: 4911
4 years ago
Philippe Antoine ae6c416972 util/mime: fix integer warnings
Ticket: 4516
4 years ago
Victor Julien ec01a94a5f detect: minor debug fixup 4 years ago
Victor Julien b7526bf4e6 decode/vntag: don't leak memory in tests 4 years ago
Victor Julien 0437ca61ff unittests: clean up packet clear logic 4 years ago
Victor Julien f07d5b2d89 decode: release refs from PacketFree
Mostly helps unittests.
4 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.
4 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.
4 years ago
Philippe Antoine a6a6f6d538 bytejump: fix ubsan warning
Instead of checking the offset, we checked the pointer after
adding the offset ot it...
4 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.
4 years ago
Jeff Lucovsky 3bd1d258a9 detect/tests: Register protoname tests
Issue: 5072

This commit registers the proto-name unit tests.
4 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.
4 years ago
Jeff Lucovsky ff0cf89738 util/proto: Protocol-name functions
Issue: 5072

This commit adds utility functions handling protocol names.
4 years ago
Jeff Lucovsky 1e2883602b error/hash: Add error code for hash add failures
Issue: 5072
4 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.
4 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.
4 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.
4 years ago
Victor Julien 8a73b242e3 detect/address: use common cidr code 4 years ago
Victor Julien 38aec1439c radix: fix unittests after stict checks 4 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.
4 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.
4 years ago
Victor Julien 311085dd34 radix: fix unittest not cleaning up 4 years ago
Victor Julien 860daceb04 detect/iponly: update SigNumArray comment 4 years ago
Victor Julien d04dface20 radix: cleanup test 4 years ago
Victor Julien 89b7ac0a60 radix: add tests for Bug #5066
Bug: #5066.
4 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.
4 years ago
Victor Julien 6381b1a643 detect/iponly: cleanups 4 years ago
Victor Julien de4354abcb detect/iponly: minor debug 'Print' improvements 4 years ago
Victor Julien 3ca3c9dfbe radix: minor debug 'Print' improvements 4 years ago
Victor Julien e04d378e58 util/cidr: simplify IPv4 CIDR handling; add IPv6
Instead of building a table at init just calculate it on demand.

Callsites are all during init, so its not performance critical.

Add similar function for IPv6.
4 years ago
Juliana Fajardini e0b9f0e175 http: add comment tags to support documentation
With these, the portion of code within the tags should be included
in the related code-snippets (for frame support documentation) w/o
errors, even if the code within changes. The tags can also work as
a reminder that the existing code is being shown elsewhere, so folks
know documentation might need updates, in case of major changes.
4 years ago
Philippe Antoine 5fe9188a95 fuzz: test for too many open txs in a flow
so as to avoid performance problems coming from this.
4 years ago
Victor Julien e1f7c63fa8 swf: fix coverity warnings
*** CID 1499365:    (UNINIT)
/src/util-file-swf-decompression.c: 98 in FileSwfZlibDecompression()
92         infstream.avail_in = (uInt)compressed_data_len;
93         infstream.next_in = (Bytef *)compressed_data;
94         infstream.avail_out = (uInt)decompressed_data_len;
95         infstream.next_out = (Bytef *)decompressed_data;
96
97         inflateInit(&infstream);

>>>     CID 1499365:    (UNINIT)
>>>     Using uninitialized value "infstream.total_out" when calling "inflate".
98         int result = inflate(&infstream, Z_NO_FLUSH);
99         switch(result) {
100             case Z_STREAM_END:
101                 break;
102             case Z_OK:
103                 break;

/src/util-file-swf-decompression.c: 98 in FileSwfZlibDecompression()
92         infstream.avail_in = (uInt)compressed_data_len;
93         infstream.next_in = (Bytef *)compressed_data;
94         infstream.avail_out = (uInt)decompressed_data_len;
95         infstream.next_out = (Bytef *)decompressed_data;
96
97         inflateInit(&infstream);

>>>     CID 1499365:    (UNINIT)
>>>     Using uninitialized value "infstream.total_out" when calling "inflate".
98         int result = inflate(&infstream, Z_NO_FLUSH);
99         switch(result) {
100             case Z_STREAM_END:
101                 break;
102             case Z_OK:
103                 break;

*** CID 1499363:  Error handling issues  (CHECKED_RETURN)
/src/util-file-swf-decompression.c: 97 in FileSwfZlibDecompression()
91
92         infstream.avail_in = (uInt)compressed_data_len;
93         infstream.next_in = (Bytef *)compressed_data;
94         infstream.avail_out = (uInt)decompressed_data_len;
95         infstream.next_out = (Bytef *)decompressed_data;
96
>>>     CID 1499363:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "inflateInit_(&infstream, "1.2.11", 112)" without checking return value. This library function may fail and return an error code.
97         inflateInit(&infstream);
98         int result = inflate(&infstream, Z_NO_FLUSH);
99         switch(result) {
100             case Z_STREAM_END:
101                 break;
102             case Z_OK:

Bug: #5079.
4 years ago
Victor Julien 4312676aed dnp3/eve: regenerate object logging code
To propagate jb_set_string_from_bytes() generator update.

Bug: #5080.
4 years ago
Jeff Lucovsky a3443845fb log/stack: Propagate original signal
Issue: 4550

This commit modifies the "stack trace on signal" to propagate the
original signal received instead of always raising SIGABRT.
4 years ago
Victor Julien 84d91e2e0d app-layer: fix counter setup logic
Completes 0ccf5b9147
4 years ago
Victor Julien 27cd54dc0d frames: address coverity issue
Minor cleanups to assist coverity.

Bug: #5065.
4 years ago
Modupe Falodun 786cf41599 detect-bytetest: remove unittests
These tests are reimplemented as Suricata-Verify

Task: 4911
4 years ago
Victor Julien c96d22e8a1 frames: support UDP frames
UDP frames point to the UDP packet payloads.

The frames are removed after each packet.

Ticket: #4983.
4 years ago
Victor Julien 97ef60cd9b output/file: remove 'waldo' code
It was no longer used after "file-store v1" was removed.
4 years ago
Victor Julien f9c04992c3 file/store: warning grammer fixup 4 years ago
Victor Julien b06bd1a1fe htp: rearrange tx user data for more efficiency 4 years ago
Victor Julien 39b1f1aca6 output/lua: minor cleanups 4 years ago
Victor Julien e5fd0d4f76 output/streaming: use unique thread data name 4 years ago
Victor Julien b36683e04f output/stats: use unique thread data name 4 years ago
Victor Julien 008f4aee69 output/packet: use unique thread data name 4 years ago
Victor Julien dd1dc88c65 output/filedata: use unique thread data name 4 years ago
Victor Julien c7db9aa50d output/file: use unique thread data name 4 years ago
Victor Julien 45f13b3d01 output/tx: use unique thread data name 4 years ago
Victor Julien 0be99f3e35 output: minor header cleanups 4 years ago
Victor Julien 645a04c233 output: declare OutputLoggerThreadStore once 4 years ago
Victor Julien 0ccf5b9147 app-layer: fix error counter logic 4 years ago
Modupe Falodun cf5c58c075 detect-uricontent: convert unittests to FAIL/PASS APIs 4 years ago
Modupe Falodun dc8908b282 detect-uricontent: remove unittests
These tests are reimplemented as Suricata-verify

Task: 4911
4 years ago
Modupe Falodun 26c9e66586 detect-engine-enip: remove unittests
These test is reimplemented in Suricata-Verify

Task: 4911
4 years ago
Victor Julien 609a7eaab2 app-layer: error counters
Per app-layer error counters for:
gap, parser, internal (AppLayerResult issues), alloc
4 years ago