[Openswan dev] LEAK_DETECTIVE hits passert()

[Paul Wouters]

On Thu, 2 Dec 2010, D. Hugh Redelmeier wrote:

> | #0  0x00002b13df8d2ceb in osw_alias_cmp (needle=0x7fff5a71acf0 "bcg/2x0",
> |     haystack=0x2aaaaab1cffc "bcg")
> |     at /builddir/build/BUILD/openswan-2.6.32rc3/lib/libwhack/aliascomp.c:50
>
> osw_alias_cmp has a bug.
>
>                s += nlen;
>                while(*s!='\0' && *s!=' ' && *s!='\t') s++;
>
> should be something like (UNTESTED):
>
>        for (;;) {
>            s++;
>            if (*s == '\0')
>                break;	/* or return FALSE: we're done */
>            if (*s == ' ' || *s == '\t') {
> 		/* at whitespace: start next scan right after */
>                s++;
>                break;
>            }
>        }

I've tested using both an old and a new style alias, using -lefence. I can confirm
that both new and old style aliases actually work (even without the patch), which
was something I was not aware of. But also, it crashes on using leftsubnetS= without
the patch, but not with the patch.

> So this code has probably never found a match that didn't start at
> offset 0 in haystack.  Why?  Because every search after the first
> starts looking at a whitespace character and that cannot match.
>
> But the actuall bug that caused the dump is that the remainder of haystack might not
> even be nlen long, so scan of s may skip the NUL at the end.
>
> Does that mean that the scan is redundant?

That's a good question. It did find the aliases (old and new style) on my test. So
why is that check there at all?

Paul