[Snort-devel] [PATCH]: Correctly detect the end of payload in base64_decode

Hui Cao (huica) huica at ...3461...
Fri Mar 7 09:59:01 EST 2014


Hi Joshhua,

Thanks for reporting this issue and providing the patch. We will address
this issue.

Best,
Hui.

On 3/7/14, 5:21 AM, "Joshua Kinard" <kumba at ...2185...> wrote:

>
>Hi snort-devel,
>
>So I ran into a curious use case w/ base64_decode, and I think it's a bug.
>In the attached patch, in Base64DecodeEval(), I modified the check for the
>end of the payload to use '>=' instead of '>'.  I had a case where a
>content
>match was at the end of the payload, but I forgot to use an
>isdataat/relative keyword before base64_decode/relative to verify that
>there
>was actually data at the end before attempting to decode.  base64_decode
>was
>relative, had no 'offset', and 'bytes' was a value >1.
>
>I thought base64_decode would've failed at this point and returned
>DETECTION_NO_MATCH, but if start_ptr == (p->data + p->dsize), it passed
>the
>condition, and somehow, Snort was either wrapping around to the beginning
>of
>the payload or meandering off somewhere else in memory.  Changing the
>condition to '>=' corrects this, and subsequent tests show the problem
>disappears if you omit an isdataat call.
>
>I also corrected a similar case in snort_plugin_api.c::base64Decode().
>
> detection-plugins/sp_base64_decode.c            |    2 +-
> dynamic-plugins/sf_engine/sf_snort_plugin_api.c |    2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
>-- 
>Joshua Kinard
>Gentoo/MIPS
>kumba at ...2185...
>4096R/D25D95E3 2011-03-28
>
>"The past tempts us, the present confuses us, the future frightens us.
>And
>our lives slip away, moment by moment, lost in that vast, terrible
>in-between."
>
>--Emperor Turhan, Centauri Republic





More information about the Snort-devel mailing list