Commit Graph

10584 Commits (afb97d1dee271b95e51a5a9985165346ca36d4ed)

Author SHA1 Message Date
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
Victor Julien ae0b8d92da flow/manager: remove dead code 4 years ago
Victor Julien 5618886aa9 stream: remove unused defines 4 years ago
Modupe Falodun d2dad66a2b detect-dce-opnum: remove unittests
These tests are reimplemented in Suricata-Verify

Task: 4911
4 years ago
Philippe Antoine 4247605d87 smtp: check if we have a current transaction
Ticket: 4948

This is not the perfect solution, but it prevents to trigger
the assert, and keep the assert.
A better solution would need to create transaction from
the reponse parsing, in case a later command was buffered and
not answered. But this would not be enough as NoNewTx prevents
the creation of a new transaction for RSET...
4 years ago
Philippe Antoine 2ef4172437 ftp: limits the number of active transactions per flow
Ticket: 4530

As for HTTP2 and MQTT.
In FTP case, transactions are pipelined, not identified by an id.
So, there are less chances of DOS by quadratic complexity.
4 years ago
Philippe Antoine b39554b11f fuzz: target for applayer cleans transactions
Ticket: 4530

Otherwise, we timeout because we kept too many of them
as Suricata would not
4 years ago
Aaron Bungay a5d3a1f92c src: use bool instead of int 4 years ago
Aaron Bungay 272786908c smtp/mime: configurable url scheme extraction
Parse extract-url-schemes from the mime config.
e.g. 'extract-urls-schemes: [http, https, ftp, mailto]'
Update MimeDecConfig struct to new url extraction fields.
Change app-layer-smtp.c & util-decode-mime.c to initialize new struct
fields for MimeDecConfig.
Sets the default value for extract-url-schemes if not found in the
config to 'extract-urls-schemes: [http]' for backwards compatibility.

Uses the schemes defined in the mime config value for
extract-urls-schemes to search for URLS starting with those scheme
names followed by "://".
Logs the URLS with the scheme + '://' at the start if the
log-url-scheme is set in the mime config, otherwise the old behaviour
is reverted to and the urls are logged with the schemes stripped.

Removed unused constant URL_STR now that URLS are being searched for
using extract-urls-schemes mime config values instead of just URL's
starting with 'http://'.

Added commented out new options for extract-urls-schemes and
log-url-scheme to suricata.yaml.in.

Update FindUrlStrings comments.
Remove old outdated comments/commented code from FindUrlStrings.
Update test case for mime which now needs schemes list to be set.
Add Test Cases for FindUrlStrings() method.

Feature: #2054
4 years ago
Modupe Falodun b77d1d7d2e detect-flowbits: remove unittests
These tests are reimplemented in Suricata-Verify

Task: 4911
4 years ago
Philippe Antoine 1e1a4ab1c4 detect: logs an error if a protocol is disabled
So that the user knows that the rule cannot match
4 years ago
Philippe Antoine bf30eb344a detect: checking validity of rules with http protocol
We want to check that a rule beginning with alert http
can be valid, that is if either HTTP1 or HTTP2 is enabled.
So, AppLayerProtoDetectGetProtoName will do a more complex
check for this ALPROTO_HTTP (any).
4 years ago
Jeff Lucovsky b53fced452 general: Fix typo 4 years ago
Jeff Lucovsky be2155b4ed config/ref: Raise errors for ref.config parsing
This commit raises an error in configuration test mode if there was an
error parsing reference.config.

Issue: 4659
4 years ago
Modupe Falodun 8d615842f9 detect/bypass: remove unittest
This test is reimplemented in Suricata-Verify

Task: 4911
4 years ago
Victor Julien 738e756eaf eve/pgsql: log txs in flow direction 4 years ago
Angelo Mirabella 41a139b590 stream-tcp-reassemble: fix reassembly direction for FIN packets
Suricata invokes the stream reassembly logic only for the current packet
direction if the packet contains a FIN flag. However, this does not
handle the case in which the packet ACKs data from the opposing direction.
This patch forces the invocation of the stream reassembly logic
on both direction when Suricata sees a FIN packet.
4 years ago
Jason Ish 9e096dda4e windows: exit early if live capture requested without npcap 4 years ago
Modupe Falodun 154e4eb395 http-response-line: remove unittest
This test is reimplemented in Suricata-Verify

Task: 4911
4 years ago
Modupe Falodun 926c02a141 detect/modbus: remove unittests
These tests are reimplemented in Suricata-Verify

Task: 4911
4 years ago
Modupe Falodun 0984528ddb detect-http-request-line: remove unittests
These tests are reimplemented as Suricata-Verify

Task: 4911
4 years ago
Modupe Falodun dff7e7d34e detect/hostbits: remove unittests
These tests are reimplemented as Suricata-Verify tests

Task: 4911
4 years ago
Modupe Falodun 47f70bf1f4 detect/proto: remove unittests
This test is reimplemented in Suricata-Verify

Task: 4911
4 years ago
Philippe Antoine 749b9c7635 fuzz: cleans all flow after one run
Completes commit e2370d6861
for all the fuzz targets processing pcaps
using a generic function.

FlowShutdown is not used because it uses the loop to destroy
mutexes, which we want to reuse for fuzzing
4 years ago
Victor Julien 40c315aa35 detect/frames: fix coverity warning
Harmless warning, but it was correct in that the code made no sense:
1497420 Dereference before null check
4 years ago
Victor Julien e902aaf838 detect/frames: fix crash when parsing bad rule
Indexing of Signature::init_data::smlists would fail for a rule that
used a frame w/o content, as the array would only be expanded when
adding a content. Adding a check to see if there list id is in bounds
is an implicit check for the "no content" case.

Bug #5011.
4 years ago
Victor Julien c6be6d2c6f detect/frames: fix error messages 4 years ago
Juliana Fajardini 0bf1227f0f pgsql: fix defect found by coverity
Pgsql was using bitwise operations to assign password output config to
its context flags, but mixing that with logic negation of the default
value, resulting in the expressions having a constant value as result.

Bug: #5007
4 years ago
Jason Ish 59ac1fe277 logging: change ownership of application log if needed
When running with privilege dropping, the application log file
is opened before privileges are dropped resulting in Suricata
failing to re-open the file for file rotation.

If needed, chown the application to the run-as user/group after
opening.

Ticker #4523
4 years ago
Jason Ish 08518df373 startup: initialize run as user info sooner
Initialize the run-as user info after loading the config, but
before setting up logging (previously it was done while initializing
signal handlers). This will allow the log file to be given the
correct permissions if Suricata is configured to run as a non-root
user.
4 years ago
Lukas Sismis f668524731 dpdk: adjust setting of MTU to the new DPDK API (21.11) 4 years ago