** CID 1257764: Dereference after null check (FORWARD_NULL)
/src/defrag.c: 291 in Defrag4Reassemble()
** CID 1257763: Dereference after null check (FORWARD_NULL)
/src/defrag.c: 409 in Defrag6Reassemble()
In the error case 'rp' can be both NULL or non-NULL.
Updated reference.config to match ET Open reference.config found here:
https://rules.emergingthreats.net/open/suricata/reference.config
Due to startup error shown here:
root@xxxxxxx01:/etc/suricata/rules# /usr/bin/suricata -c /etc/suricata/suricata.yaml --pidfile /var/run/suricata.pid --af-packet
23/12/2014 -- 22:07:56 - <Error> - [ERRCODE: SC_ERR_REFERENCE_UNKNOWN(150)] - unknown reference key "osvdb". Supported keys are defined in reference.config file. Please have a look at the conf param "reference-config-file"
<...>
Killed
This patches is fixing a issue in the OutputJSONBuffer function. It
was writing to file the content of the buffer starting from the start
to the final offset. But as the writing is done for each JSON string
we are duplicating the previous events if we are reusing the same
buffer.
Duplication was for example triggered when we have multiple alerts
attached to a packet. In the case of two alerts, the first one was
logged twice more as the second one.
This si almost the same code as the one of master but it fixes a
conflict during cherry picking in:
src/output-json-alert.c
Due to a logic error in AppLayerProtoDetectGetProtoByName invalid
protocols would not be detected as such. Instead of ALPROTO_UNKNOWN
ALPROTO_MAX was returned.
Bug #1329
This patch fixes the following errors:
[src/unix-manager.c:306]: (error) Memory pointed to by 'client' is freed twice.
[src/unix-manager.c:313]: (error) Memory pointed to by 'client' is freed twice.
[src/unix-manager.c:323]: (error) Memory pointed to by 'client' is freed twice.
[src/unix-manager.c:334]: (error) Memory pointed to by 'client' is freed twice.
Unix manager was treating the packet after closing the socket if message was
too long.
The global variable suricata_ctl_flags needs to volatile, otherwise the
compiler might not cause the variable to be read every time because it
doesn't know other threads might write the variable.
This was causing Suricata to not exit under some conditions.
For these 2 cases:
1. Missing SYN:
-> syn <= missing
<- syn/ack
-> ack
-> data
2. Missing SYN and 3whs ACK:
-> syn <= missing
<- syn/ack
-> ack <= missing
-> data
Fix session pickup. The next_win settings weren't correctly set, so that
packets were rejected.
Bug 1190.
If 3whs SYN/ACK and ACK are missing we can still pick up the session if
in async-oneside mode.
-> syn
<- syn/ack <= missing
-> ack <= missing
-> data
Bug 1190.
ACK packets completing a valid FIN shutdown could be flagged as
'bad window update' if they would shrink the window.
This patch detects this case before doing the bad window update
check.
If the 3whs ACK and some data after this is lost, we would get stuck
in the 'SYN_RECV' state, where from there each packet might be
considered invalid.
This patch improves the handling of this case.
The keyword would not allow matching on "OpenSSH_5.5p1 Debian-6+squeeze5"
as the + and space characters were not allowed.
This patch adds support for them.
The base 'lua' library has different names on different OS' and even
Linux distro's. Instead of selecting the proper one, we now just try
all. This way no OS/distro specific knowledge about the name is needed.
A logic error in the IPv6 Routing header parsing caused accidental
updating of the original packet buffer. The calculated extension
header lenght was set to the length field of the routing header,
causing it to be wrong.
This has 2 consequences:
1. defrag failure. As the now modified payload was used in defrag,
the decoding of the reassembled packet now contained a broken length
field for the routing header. This would lead to decoding failure.
The potential here is evasion, although it would trigger:
[1:2200014:1] SURICATA IPv6 truncated extension header
2. in IPS mode, especially the AF_PACKET mode, the modified and now
broken packet would be transmitted on the wire. It's likely that
end hosts and/or routers would reject this packet.
NFQ based IPS mode would be less affected, as it 'verdicts' based on
the packet handle. In case of replacing the packet (replace keyword
or stream normalization) it could broadcast the bad packet.
Additionally, the RH Type 0 address parsing was also broken. It too
would modify the original packet. As the result of this code was not
used anywhere else in the engine, this code is now disabled.
Reported-By: Rafael Schaefer <rschaefer@ernw.de>
AF_PACKET is not setting the engine mode to IPS when some
interfaces are peered and use IPS mode. This is due to the
fact, it is possible to peer 2 interfaces and run an IPS on
them and have a third one that is running in normal IDS mode.
In fact this choice is the bad one as unwanted side effect is
that there is no drop log and that stream inline is not used.
To fix that, this patch puts suricata in IPS mode as soon as
there is two interfaces in IPS mode. And it displays a error
message to warn user that the accuracy of detection on IDS only
interfaces will be low.
The request line scripts were added to the AMATCH list. However, there
is not AppLayerMatch function defined for lua scripts. So scripts
would not run.
This patch adds the request line scripts to the normal 'MATCH' list.
Bug #1273.
I found three somewhat serious IPv6 address bugs within the Suricata 2.0.x source code. Two are in the source module "detect-engine-address.c", and the third is in "util-radix-tree.c".
The first bug occurs within the function DetectAddressParse2(). When parsing an address string and a negated block is encountered (such as when parsing !$HOME_NET, for example), any corresponding IPv6 addresses were not getting added to the Group Heads in the DetectAddressList. Only IPv4 addresses were being added.
I discovered another bug related to IPv6 address ranges in the Signature Match Address Array comparison code for IPv6 addresses. The function DetectAddressMatchIPv6() walks a signature's source or destination match address list comparing each to the current packet's corresponding address value. The match address list consists of value pairs representing a lower and upper IP address range. If the packet's address is within that range (including equal to either the lower or upper bound), then a signature match flag is returned.
The original test of each signature match address to the packet was performed using a set of four compounded AND comparisons looking at each of the four 32-bit blocks that comprise an IPv6 address. The problem with the old comparison is that if ANY of the four 32-bit blocks failed the test, then a "no-match" was returned. This is incorrect. If one or more of the more significant 32-bit blocks met the condition, then it is a match no matter if some of the less significant 32-bit blocks did not meet the condition. Consider this example where Packet represents the packet address being checked, and Target represents the upper bound of a match address pair. We are testing if Packet is less than Target.
Packet -- 2001:0470 : 1f07:00e2 : 1988:01f1 : d468:27ab
Target -- 2001:0470 : 1f07:00e2 : a48c:2e52 : d121:101e
In this example the Packet's address is less than the target and it should give a match. However, the old code would compare each 32-bit block (shown spaced out above for clarity) and logically AND the result with the next least significant block comparison. If any of the four blocks failed the comparison, that kicked out the whole address. The flaw is illustrated above. The first two blocks are 2001:0470 and 1f07:00e2 and yield TRUE; the next less significant block is 1988:01f1 and a48c:2e52, and also yields TRUE (that is, Packet is less than Target); but the last block compare is FALSE (d468:27ab is not less than d121:101e). That last block is the least significant block, though, so its FALSE determination should not invalidate a TRUE from any of the more significant blocks. However, in the previous code using the compound logical AND block, that last least significant block would invalidate the tests done with the more significant blocks.
The other bug I found for IPv6 occurs when trying to parse and insert an IPv6 address into a Radix Tree using the function SCRadixAddKeyIPV6String(). The test for min and max values for an IPv6 CIDR mask incorrectly tests the upper limit as 32 when it should be 128 for an IPv6 address. I think this perhaps is an old copy-paste error if the IPv6 version of this function was initially copied from the corresponding IPv4 version directly above it in the code. Without this patch, the function will return null when you attempt to add an IPv6 network whose CIDR mask is larger than 32 (for example, the popular /64 mask will cause the function to return the NULL error condition).
(amended by Victor Julien)
Reported in bug 1238 is an issue where stream reassembly can be
disrupted.
A packet that was in-window, but otherwise unexpected set the
window to a really low value, causing the next *expected* packet
to be considered out of window. This lead to missing data in the
stream reassembly.
The packet was unexpected in various ways:
- it would ack unseen traffic
- it's sequence number would not match the expected next_seq
- set a really low window, while not being a proper window update
Detection however, it greatly hampered by the fact that in case of
packet loss, quite similar packets come in. Alerting in this case
is unwanted. Ignoring/skipping packets in this case as well.
The logic used in this patch is as follows. If:
- the packet is not a window update AND
- packet seq > next_seq AND
- packet acq > next_seq (packet acks unseen data) AND
- packet shrinks window more than it's own data size
THEN set event and skip the packet in the stream engine.
So in case of a segment with no data, any window shrinking is rejected.
Bug #1238.