Bug #5223
closedbase64_decode does not populate base64_data buffer once hitting non-base64 chars
Description
consider the following rules and the attached pcap.
The rules are designed to test the behavior of when non-base64 characters are encountered by the base64_decode.
Pay particular attention to sid:4 and sid:2 where the only difference is how far into the base64 encoded string are decoded.
if the base64_decode was populating the base64_data buffer with data upto the non-base64 char, we expect the first byte of the base64 decoded value (|9d|
) to be populated into base64_data
alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"base64 decode - no url_decode"; flow:established,to_server; http.cookie; content:"foobar="; base64_decode:relative; base64_data; content:"|9e|"; startswith; sid:1; rev:1;) alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"base64 decode - url_decode"; flow:established,to_server; http.cookie; url_decode; content:"foobar="; base64_decode:relative; base64_data; content:"|9e|"; sid:2; rev:1;) alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"base64 decode - no url_decode, proves no base64_data buffer via pcre"; flow:established,to_server; http.cookie; content:"foobar="; base64_decode:relative; base64_data; pcre:"/./"; sid:3; rev:1;) alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"base64 decode - no url_decode grab only the first two bytes"; flow:established,to_server; http.cookie; content:"foobar="; base64_decode:bytes 2,relative; base64_data; content:"|9e|"; startswith; sid:4; rev:1;)
Files
Updated by Brandon Murphy over 2 years ago
correction:
Pay particular attention to sid:4 and sid:2 where the only difference is how far into the base64 encoded string are decoded.
should be
Pay particular attention to sid:4 and sid:1 where the only difference is how far into the base64 encoded string are decoded.
in testing, the following alerts are produced
03/30/2022-14:59:10.243824 [**] [1:2:1] base64 decode - url_decode [**] [Classification: (null)] [Priority: 3] {TCP} 192.168.104.20:12926 -> 172.16.72.94:80 03/30/2022-14:59:10.243824 [**] [1:4:1] base64 decode - no url_decode grab only the first two bytes [**] [Classification: (null)] [Priority: 3] {TCP} 192.168.104.20:12926 -> 172.16.72.94:80
Updated by Brandon Murphy over 2 years ago
the following rule, when run on snort 2.9.18 (only tested version) works as expected. this is equivalent to sid:1; of the above suricata rules.
alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"base64 decode"; flow:established,to_server; content:"foobar="; http_cookie; base64_decode:relative; base64_data; content:"|9e|"; depth:1; sid:1; rev:1;)
[**] [1:1:1] base64 decode [**] [Priority: 0] 03/30-14:59:10.246444 192.168.104.20:12926 -> 172.16.72.94:80 TCP TTL:64 TOS:0x0 ID:1 IpLen:20 DgmLen:125 ***AP*** Seq: 0xB Ack: 0x65 Win: 0x2000 TcpLen: 20F
Updated by Victor Julien over 2 years ago
- Status changed from New to Assigned
- Assignee changed from OISF Dev to Shivani Bhardwaj
- Priority changed from Normal to High
- Target version changed from TBD to 7.0.0-beta1
Shivani could you craft an SV test for this and have a look at the issue? AFAICS we could introduce a new Base64Mode
like BASE64_MODE_RELAX_PASSTROUGH
that would just copy any invalid byte into dest
. We should be careful with the buffer sizes though, as the dest buffer might be smaller than expected due to there normally being a size ratio between encoded and decoded data.
Updated by Brandon Murphy over 2 years ago
- File 2bfb4e704fd597e.pcap 2bfb4e704fd597e.pcap added
Updated pcap as Shivani pointed out the 97b97bef320a2ea.pcap was missing the hose host header and resulted in anomaly alerts.
Updated by Shivani Bhardwaj over 2 years ago
- Status changed from Assigned to In Progress
Updated by Shivani Bhardwaj about 2 years ago
Victor Julien wrote in #note-3:
Shivani could you craft an SV test for this and have a look at the issue? AFAICS we could introduce a new
Base64Mode
likeBASE64_MODE_RELAX_PASSTROUGH
that would just copy any invalid byte intodest
. We should be careful with the buffer sizes though, as the dest buffer might be smaller than expected due to there normally being a size ratio between encoded and decoded data.
I was looking at this with this approach and realized that dest buffer would indeed be a problem but wondered why should we try to add some data that is not decoded to the decoded buffer?
I referred to the RFC again and it is advisable for the decoding software to skip on any such invalid chars in a stream of b64 encoded data. So, I was wondering if it makes sense to make our decoder such that it always skips on anything that is not supposed to be there?
Even if I try to put the string in this pcap on any reliable decoder, the results are the same as they would be if there were no special characters..
But, I am clearly missing the point of BASE64_MODE_RELAX
while proposing this (which I don't know btw)..
Updated by Shivani Bhardwaj almost 2 years ago
- Status changed from In Progress to In Review
Updated by Shivani Bhardwaj almost 2 years ago
- Status changed from In Review to Closed
Closed by: https://github.com/OISF/suricata/pull/8042
Updated by Shivani Bhardwaj almost 2 years ago
- Status changed from Closed to Resolved
Updated by Shivani Bhardwaj almost 2 years ago
- Label deleted (
Needs backport to 6.0)
Updated by Victor Julien almost 2 years ago
- Status changed from Resolved to Closed