Update: [PATCH 1/2] Add a function to remove an element/token from a header, value (like Accept-Encoding from Vary:)

Poul-Henning Kamp phk at phk.freebsd.dk
Mon Sep 22 09:37:20 CEST 2014


--------
In message <5419C6B1.9010305 at schokola.de>, 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
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.



More information about the varnish-dev mailing list