This is the third patch.
In this patch also solved a small buffer overread which exist in the
original httpHeaderParseQuotedString function. The loop:
while (end <= (start+len) && *end != '\\' && *end != '\"' && *end >
0x1F && *end != 0x7F)
if (*end <= 0x1F || *end == 0x7F) {
...
allowed to access (and parsing affected by) the char after the end of
parsed string. It did not have any bad effect for null terminated strings.
On 05/27/2011 03:12 PM, Amos Jeffries wrote:
On 27/05/11 23:21, Tsantilas Christos wrote:
Hi all,
Just trying to clarify what we want to implement at the end, because I
am confused. I am responsible for the confusion because I give two "(3)"
options, and I send buggy implementations for the "(1)" and the "second
(3)" option.
From what I can understand, currently, we have the following options:
1) Just ignore any "\r" or "\n" character. This is the fastest and
simpler approach
2) Require "[\r]\n " or "[\r]\n\t" as line separator and replace it with
a space.
From the discussion the (1) may be dangerous because strings like this
"1\r23" will be converted to "123" which maybe it is dangerous.
So I suppose we should implement the (2) option. Is it OK?
Agreed.
What we have been debugging in the other half of the thread was "\r\n "
or "\r\n\t".
I think it just needs:
* the two buffer overread bugs Alex spotted removed,
* the \r made optional.
Amos
=== modified file 'src/HttpHeaderTools.cc'
--- src/HttpHeaderTools.cc 2010-11-02 00:12:43 +0000
+++ src/HttpHeaderTools.cc 2011-05-30 13:29:36 +0000
@@ -325,60 +325,91 @@
/**
* Parses a quoted-string field (RFC 2616 section 2.2), complains if
* something went wrong, returns non-zero on success.
* Un-escapes quoted-pair characters found within the string.
* start should point at the first double-quote.
*/
int
httpHeaderParseQuotedString(const char *start, const int len, String *val)
{
const char *end, *pos;
val->clean();
if (*start != '"') {
debugs(66, 2, "failed to parse a quoted-string header field near '" << start << "'");
return 0;
}
pos = start + 1;
while (*pos != '"' && len > (pos-start)) {
+
+ if (*pos =='\r') {
+ pos++;
+ if ((pos-start) > len || *pos != '\n') {
+ debugs(66, 2, "failed to parse a quoted-string header field with \'\\r\' octet " << (start-pos)
+ << " bytes into '" << start << "'");
+ val->clean();
+ return 0;
+ }
+ }
+
+ if (*pos == '\n') {
+ pos++;
+ if ( (pos-start) > len || (*pos != ' ' && *pos != '\t')) {
+ debugs(66, 2, "failed to parse multiline quoted-string header field '" << start << "'");
+ val->clean();
+ return 0;
+ }
+ val->append(" ");
+ pos++;
+ debugs(66, 2, "len < pos-start => " << len << " < " << (pos-start));
+ continue;
+ }
+
bool quoted = (*pos == '\\');
- if (quoted)
+ if (quoted) {
pos++;
- if (!*pos || (pos-start) > len) {
- debugs(66, 2, "failed to parse a quoted-string header field near '" << start << "'");
- val->clean();
- return 0;
+ if (!*pos || (pos-start) > len) {
+ debugs(66, 2, "failed to parse a quoted-string header field near '" << start << "'");
+ val->clean();
+ return 0;
+ }
}
end = pos;
- while (end <= (start+len) && *end != '\\' && *end != '\"' && *end > 0x1F && *end != 0x7F)
+ while (end < (start+len) && *end != '\\' && *end != '\"' && *end > 0x1F && *end != 0x7F)
end++;
- if (*end <= 0x1F || *end == 0x7F) {
+ if ((*end <= 0x1F && *end != '\r' && *end != '\n') || *end == 0x7F) {
debugs(66, 2, "failed to parse a quoted-string header field with CTL octet " << (start-pos)
<< " bytes into '" << start << "'");
val->clean();
return 0;
}
val->append(pos, end-pos);
pos = end;
}
+
+ if (*pos != '\"') {
+ debugs(66, 2, "failed to parse a quoted-string header field which did not end with \" ");
+ val->clean();
+ return 0;
+ }
/* Make sure it's defined even if empty "" */
if (!val->defined())
val->limitInit("", 0);
return 1;
}
/**
* Checks the anonymizer (header_access) configuration.
*
* \retval 0 Header is explicitly blocked for removal
* \retval 1 Header is explicitly allowed
* \retval 1 Header has been replaced, the current version can be used.
* \retval 1 Header has no access controls to test
*/
static int
httpHdrMangle(HttpHeaderEntry * e, HttpRequest * request, int req_or_rep)
{
int retval;
/* check with anonymizer tables */
