[tcpdump-workers] ND_TTEST2 --- can l be negative

[PEUGNEZ Baptiste]

It's funny, I have finished working on this issue yesterday. I detected 
with a fuzzer.
I fixed it like this:

#define ND_TTEST2(var, l) ((uintptr_t)(ndo->ndo_snapend >= (l) && \
                         (const u_char *)&(var) <= ndo->ndo_snapend - (l))

With a debug function in the file "print-snmp.c"

static int
debug_check(netdissect_options *ndo,
            struct be *elem)
{
         u_char *p = (u_char *)elem->data.raw;
         uint32_t asnlen = elem->asnlen;

         printf("p: %lx \n",p);
         printf("*p: %c \n",*p);
         printf("&p: %lx \n",&p);
         printf("asnlen: %x \n", asnlen);
         printf("elem->data.raw: %lx \n",elem->data.raw);
         printf("ndo->ndo_snapend: %lx \n",ndo->ndo_snapend);
         printf("ndo->ndo_snapend - asnlen: %lx \n",ndo->ndo_snapend - 
asnlen);
         printf("ndo->ndo_snapend - (l) <= ndo->ndo_snapend: %d 
\n",ndo->ndo_snapend - asnlen <= ndo->ndo_snapend);
         printf("ndo->ndo_snapend: %lx \n",(uintptr_t)ndo->ndo_snapend);
         printf("ndo->ndo_snapend - asnlen: %lx 
\n",(uintptr_t)(ndo->ndo_snapend - asnlen));
         printf("ndo->ndo_snapend - (l) >= 0: %d 
\n",(uintptr_t)ndo->ndo_snapend >= asnlen);
}

static int
asn1_print(netdissect_options *ndo,
            struct be *elem)
{
         u_char *p = (u_char *)elem->data.raw;
         uint32_t asnlen = elem->asnlen;
         uint32_t i;

         switch (elem->type) {

         case BE_OCTET:
                 debug_check(ndo, elem);
                 ND_TCHECK2(*p, asnlen);
                 #if 0
                 for (i = asnlen; i-- > 0; p++)
                         ND_PRINT((ndo, "_%.2x", *p));
                 #endif
                 break;
....

On 03/09/15 19:23, Michael Richardson wrote:
> What do you think of this patch.
> I was trying to use ND_TTEST2() in a place where l was seemingly to become
> negative, and I felt that we should check for that situation.
> In the situation at hand, that actually wasn't the problem, but I still think
> that perhaps this is a good thing.
>
>
> --- a/netdissect.h
> +++ b/netdissect.h
> @@ -271,8 +271,9 @@ struct netdissect_options {
>    *     http://www.kb.cert.org/vuls/id/162289
>      */
>       #define ND_TTEST2(var, l) \
>       +  ((l) >= 0 && \
>               ((uintptr_t)ndo->ndo_snapend - (l) <=
> (uintptr_t)ndo->ndo_snapend && \
> -           (uintptr_t)&(var) <= (uintptr_t)ndo->ndo_snapend - (l))
> +         (uintptr_t)&(var) <= (uintptr_t)ndo->ndo_snapend - (l)))
>
>   /* True if "var" was captured */
>    #define ND_TTEST(var) ND_TTEST2(var, sizeof(var))
>    
>
> _______________________________________________
> tcpdump-workers mailing list
> tcpdump-workers at lists.tcpdump.org
> https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers