Re: snort2: curious code in rfc5987_attr_char()

--===============1378488737289373000==
Content-Type: multipart/alternative; boundary="0000000000001ecde3059a16dd80"

--0000000000001ecde3059a16dd80
Content-Type: text/plain; charset="UTF-8"

Good catch Bob - the OR's combined with the negates means it always returns
true, thus nullifying the check in the first place.



On Thu, Dec 19, 2019 at 3:17 PM Bob Cook via Snort-devel <
[email protected]> wrote:

> Hello,
>
> I've been reviewing a little bit of the snort code and this got flagged by
> a static analyzer:
>
> src\preprocessors\snort_httpinspect.c:3891 (snort-2.9.15)
>
> static inline bool rfc5987_attr_char(u_char c)
> {
>     return rfc_2616_token(c) && ((c != '*') || (c != '\'') || (c != '%'));
> }
>
> The logical OR operators are likely supposed to be logical AND operators,
> which would make way more sense given the definition of RFC 5987.
>
> Anyone else noticed this before? The code isn't new, seems like it's been
> around for a while and still exists in 2.9.15.
>
> The side effect of this behavior would appear to be pretty minor, as this
> function is only used in parsing the Content-Disposition header, but
> figured it's still worth pointing out / asking about. There may be some
> history that I'm not aware of.
>
> Thanks,
> Bob Cook
> Sophos
>
> _______________________________________________
> Snort-devel mailing list
> [email protected]
> https://lists.snort.org/mailman/listinfo/snort-devel
>
> Please visit http://blog.snort.org for the latest news about Snort!
>


-- 
Dave Killion

--0000000000001ecde3059a16dd80
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Good catch Bob - the OR&#39;s combined with the negates me=
ans it always returns true, thus nullifying the check in the first place.<d=
iv><br></div><div><br></div></div><br><div class=3D"gmail_quote"><div dir=
=3D"ltr" class=3D"gmail_attr">On Thu, Dec 19, 2019 at 3:17 PM Bob Cook via =
Snort-devel &lt;<a href=3D"mailto:[email protected]">snort-devel@=
lists.snort.org</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" s=
tyle=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);pad=
ding-left:1ex">Hello,<br>
<br>
I&#39;ve been reviewing a little bit of the snort code and this got flagged=
 by a static analyzer:<br>
<br>
src\preprocessors\snort_httpinspect.c:3891 (snort-2.9.15)<br>
<br>
static inline bool rfc5987_attr_char(u_char c)<br>
{<br>
=C2=A0 =C2=A0 return rfc_2616_token(c) &amp;&amp; ((c !=3D &#39;*&#39;) || =
(c !=3D &#39;\&#39;&#39;) || (c !=3D &#39;%&#39;));<br>
}<br>
<br>
The logical OR operators are likely supposed to be logical AND operators, w=
hich would make way more sense given the definition of RFC 5987.<br>
<br>
Anyone else noticed this before? The code isn&#39;t new, seems like it&#39;=
s been around for a while and still exists in 2.9.15.<br>
<br>
The side effect of this behavior would appear to be pretty minor, as this f=
unction is only used in parsing the Content-Disposition header, but figured=
 it&#39;s still worth pointing out / asking about. There may be some histor=
y that I&#39;m not aware of.<br>
<br>
Thanks,<br>
Bob Cook<br>
Sophos<br>
<br>
_______________________________________________<br>
Snort-devel mailing list<br>
<a href=3D"mailto:[email protected]" target=3D"_blank">Snort-deve=
[email protected]</a><br>
<a href=3D"https://lists.snort.org/mailman/listinfo/snort-devel" rel=3D"nor=
eferrer" target=3D"_blank">https://lists.snort.org/mailman/listinfo/snort-d=
evel</a><br>
<br>
Please visit <a href=3D"http://blog.snort.org" rel=3D"noreferrer" target=3D=
"_blank">http://blog.snort.org</a> for the latest news about Snort!<br>
</blockquote></div><br clear=3D"all"><div><br></div>-- <br><div dir=3D"ltr"=
 class=3D"gmail_signature">Dave Killion<br></div>

--0000000000001ecde3059a16dd80--

--===============1378488737289373000==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Snort-devel mailing list
[email protected]
https://lists.snort.org/mailman/listinfo/snort-devel

Please visit http://blog.snort.org for the latest news about Snort!

--===============1378488737289373000==--