[Xapian-tickets] [Xapian] #578: omega GET query parsing defects and fix
Xapian
nobody at xapian.org
Mon Nov 14 05:45:57 GMT 2011
#578: omega GET query parsing defects and fix
--------------------+-------------------------------------------------------
Reporter: catkin | Owner: olly
Type: defect | Status: new
Priority: normal | Milestone: 1.2.8
Component: Omega | Version: 1.2.7
Severity: normal | Keywords:
Blockedby: | Platform: All
Blocking: |
--------------------+-------------------------------------------------------
Changes (by olly):
* milestone: => 1.2.8
Old description:
> Without this patch:
> - if the QUERY_STRING ends with a '%', the parsing routine
> will overrun the QUERY_STRING (if it is followed in memory
> by only a single NUL byte).
> - omega stops parsing GET query parameters if there are
> two consecutive ampersands in QUERY_STRING.
> - omega tries to parse anything following a percent sign.
> Other implementations do not try to parse malformed escape
> sequences.
>
> The patch is also attached as a file
> ----
> diff -Naur xapian-omega-1.2.7-original/cgiparam.cc xapian-
> omega-1.2.7/cgiparam.cc
> --- xapian-omega-1.2.7-original/cgiparam.cc 2011-08-10
> 09:49:12.000000000 +0300
> +++ xapian-omega-1.2.7/cgiparam.cc 2011-11-13 08:10:15.021566998
> +0200
> @@ -180,20 +180,28 @@
> while (1) {
> ch = *q_str++;
> if (ch == '\0' || ch == '&') {
> - if (name.empty()) return; // end on blank line
> - add_param(name, val);
> + if (!name.empty()) add_param(name, val);
> + if (ch == '\0')
> + return;
> break;
> }
> char orig_ch = ch;
> if (ch == '+')
> ch = ' ';
> - else if (ch == '%') {
> - int c = *q_str++;
> - ch = (c & 0xf) + ((c & 64) ? 9 : 0);
> - if (c) c = *q_str++;
> - ch = ch << 4;
> - ch |= (c & 0xf) + ((c & 64) ? 9 : 0);
> - if (!c) return; // unfinished % code
> + else if (ch == '%' &&
> + ((q_str[0] >= '0' && q_str[0] <= '9') ||
> + (q_str[0] >= 'A' && q_str[0] <= 'F') ||
> + (q_str[0] >= 'a' && q_str[0] <= 'f')) &&
> + ((q_str[1] >= '0' && q_str[1] <= '9') ||
> + (q_str[1] >= 'A' && q_str[1] <= 'F') ||
> + (q_str[1] >= 'a' && q_str[1] <= 'f'))) {
> + const int c1 = q_str[0], c2 = q_str[1];
> + int c;
> + c = ( (c1 & 0xf) + ((c1 & 64) ? 9 : 0) ) << 4
> + + ( (c2 & 0xf) + ((c2 & 64) ? 9 : 0) );
> + q_str += 2;
> + if (!c)
> + continue;
> }
> if (had_equals) {
> val += char(ch);
New description:
Without this patch:
- if the QUERY_STRING ends with a '%', the parsing routine
will overrun the QUERY_STRING (if it is followed in memory
by only a single NUL byte).
- omega stops parsing GET query parameters if there are
two consecutive ampersands in QUERY_STRING.
- omega tries to parse anything following a percent sign.
Other implementations do not try to parse malformed escape
sequences.
The patch is also attached as a file
----
{{{
diff -Naur xapian-omega-1.2.7-original/cgiparam.cc xapian-
omega-1.2.7/cgiparam.cc
--- xapian-omega-1.2.7-original/cgiparam.cc 2011-08-10
09:49:12.000000000 +0300
+++ xapian-omega-1.2.7/cgiparam.cc 2011-11-13 08:10:15.021566998
+0200
@@ -180,20 +180,28 @@
while (1) {
ch = *q_str++;
if (ch == '\0' || ch == '&') {
- if (name.empty()) return; // end on blank line
- add_param(name, val);
+ if (!name.empty()) add_param(name, val);
+ if (ch == '\0')
+ return;
break;
}
char orig_ch = ch;
if (ch == '+')
ch = ' ';
- else if (ch == '%') {
- int c = *q_str++;
- ch = (c & 0xf) + ((c & 64) ? 9 : 0);
- if (c) c = *q_str++;
- ch = ch << 4;
- ch |= (c & 0xf) + ((c & 64) ? 9 : 0);
- if (!c) return; // unfinished % code
+ else if (ch == '%' &&
+ ((q_str[0] >= '0' && q_str[0] <= '9') ||
+ (q_str[0] >= 'A' && q_str[0] <= 'F') ||
+ (q_str[0] >= 'a' && q_str[0] <= 'f')) &&
+ ((q_str[1] >= '0' && q_str[1] <= '9') ||
+ (q_str[1] >= 'A' && q_str[1] <= 'F') ||
+ (q_str[1] >= 'a' && q_str[1] <= 'f'))) {
+ const int c1 = q_str[0], c2 = q_str[1];
+ int c;
+ c = ( (c1 & 0xf) + ((c1 & 64) ? 9 : 0) ) << 4
+ + ( (c2 & 0xf) + ((c2 & 64) ? 9 : 0) );
+ q_str += 2;
+ if (!c)
+ continue;
}
if (had_equals) {
val += char(ch);
}}}
--
Comment:
Thanks.
We should fix these for 1.2.8.
--
Ticket URL: <http://trac.xapian.org/ticket/578#comment:1>
Xapian <http://xapian.org/>
Xapian
More information about the Xapian-tickets
mailing list