Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Base64 backports 6.0.x/v2 #9292

Merged
merged 9 commits into from
Jul 28, 2023

Conversation

inashivb
Copy link
Member

@inashivb inashivb commented Jul 26, 2023

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/6224

SV_BRANCH=OISF/suricata-verify#1332

Previous PR: #9289

Changes since v1:

  • use --debug-failed with s-v runs

RFC 2045 states that any invalid character should be skipped over, this
is the RFC used by mime handler in Suricata code to deal with base64
encoded data.
So far, only spaces were skipped as a part of implementation of this
RFC, extend it to also skip over any other invalid character. Add
corresponding test.

(cherry picked from commit e46b033)
Padding bytes for the last remainder data should be as follows:

Case   |    Remainder bytes     |    Padding
----------------------------------------------
  I    |              1         |      3
  II   |              2         |      2
  III  |              3         |      1

However, we calculate the decoded_bytes with the formula:
decoded_bytes = ASCII_BLOCK - padding

this means for Case I when padding is 3 bytes, the decoded_bytes would
be 0. This is incorrect for any trailing data. In any of the above
cases, if the parsing was successful, there should at least be 1 decoded
byte.

(cherry picked from commit 095c335)
Just like the check for destination buffer size done previously for
complete data, it should also be done for the trailing data to avoid
goind out of bounds.

(cherry picked from commit 0e8b451)
The destination buffer should be able to hold at least 3 Bytes during
the processing of the last block of data. If it cannot hold at least 3
Bytes, then that may lead to dynamic buffer overflow while decoding.

(cherry picked from commit 62d7821)
(cherry picked from commit 49ad383)
For certain edge case handling for spaces, spaces were handled
particularly in the remainder processing functions. Make sure that now
that as per RFC 2045, util-base64 would skip over any invalid char, the
edge cases in MIME processor also be handled the same way.

This completes the work done in e46b033.

Ticket 6135
Ticket 6207

(cherry picked from commit 789353b)
@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.flow.spare 2037891 2166307 106.3%

Pipeline 15396

@inashivb inashivb marked this pull request as ready for review July 27, 2023 14:36
@inashivb inashivb requested review from victorjulien and a team as code owners July 27, 2023 14:37
@victorjulien victorjulien merged commit 77faa88 into OISF:master-6.0.x Jul 28, 2023
50 checks passed
@inashivb inashivb deleted the base64-backports-6.0.x/v2 branch July 28, 2023 05:29
@inashivb inashivb restored the base64-backports-6.0.x/v2 branch November 22, 2023 06:25
@inashivb inashivb deleted the base64-backports-6.0.x/v2 branch March 6, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants