--------
In message <[email protected]>, Nils Goroll writes:

>With this change, http_RemoveHdrToken also removes any values of an element
>token seperated by equals (like foo=bar)
>
>phk, does this work for you?

Question:  What if the target token appears more than once ?

Overall I find the function much more complex than I like.  I think it
is because the answer http_GetHdrToken() gives is not what we need to
know.

I would do it something like this instead:

        did = 0
        Reserve workspace
        loop over tokens in src-header
                if token != target
                        copy token to workspace
                        did = 1
        if (did):
                release unused workspace
        else:
                return all reserved workspace
        return (did)

That would largely be a duplication of http_GetHdrToken() just a few
lines longer.

When we want to do more token-based functions, we should probably make
some foreach_token() infrastructure, but lets wait until #3 case crops
up...


Two security related observations:

>+      if (! (http_GetHdr(hp, hdr, &bp) &&
>+              http_GetHdrToken(hp, hdr, token, &tp)))
>+              return (0);
>+
>+      /* GetHdrToken returns a pointer to the next char after the token */
>+      tl = strlen(token);

Subtracting from pointers is a prime minefield for security issues,
and therefore I generally try to avoid it at all cost.  If I have
to do it, I try to plug in asserts which A) show that we know what
we're doing and B) stop us of we were wrong about that.

So here I would add:

        assert(tp - tl >= bp);

>+      tp -= tl;


This one is a big NO-NO:

>+              VSLb(hp->vsl, SLT_LostHeader, hdr);

You must *always* supply a const char* format string:

>+              VSLb(hp->vsl, SLT_LostHeader, "%s", hdr);

Otherwise a header with '%' in it will make the printf access random
memory.

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
[email protected]         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.

_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Reply via email to