Hi Dave,
The patch below corrects a buffer overflow bug in q_parser.y. Since it
is triggered by excessively long query strings, I believe that this bug
could be exploited to allow arbitrary code execution if a query string
supplied by a user is passed in directly to Ferret and not truncatated.
If I''m right, you should consider a new release asap.
I''ve fixed it to simply allocate more memory if the standard buffers
aren''t enough (because I had some long (i.e. > 255) query strings
that I
needed to support), but there are other solutions as well.
Index: c/include/search.h
==================================================================---
c/include/search.h (revision 615)
+++ c/include/search.h (working copy)
@@ -819,6 +819,7 @@
char *qstr;
char *qstrp;
char buf[QP_CONC_WORDS][MAX_WORD_SIZE];
+ char *dynbuf;
int buf_index;
HashTable *field_cache;
HashSet *fields;
Index: c/src/q_parser.y
==================================================================---
c/src/q_parser.y (revision 615)
+++ c/src/q_parser.y (working copy)
@@ -173,6 +173,11 @@
char *bufp = buf;
qp->buf_index = (qp->buf_index + 1) % QP_CONC_WORDS;
+ if (qp->dynbuf) {
+ free(qp->dynbuf);
+ qp->dynbuf = NULL;
+ }
+
qp->qstrp--; /* need to back up one character */
while (!strchr(not_word, (c=*qp->qstrp++))) {
@@ -192,6 +197,15 @@
default:
*bufp++ = c;
}
+ /* we''ve exceeded the static buffer. switch to the dynamic
+ one. */
+
+ if (!qp->dynbuf && ((bufp - buf) == MAX_WORD_SIZE)) {
+ qp->dynbuf = calloc(strlen(qp->qstr) + 1, sizeof(char));
+ strncpy(qp->dynbuf, buf, MAX_WORD_SIZE);
+ buf = qp->dynbuf;
+ bufp = buf + MAX_WORD_SIZE;
+ }
}
qp->qstrp--;
/* check for keywords. There are only four so we have a bit of a hack which
@@ -262,7 +276,7 @@
}
mutex_unlock(&qp->mutex);
RAISE(PARSE_ERROR, "couldn''t parse query
``%s''''. Error message "
- " was %se", buf, (char *)msg);
+ "was: %s", buf, (char *)msg);
}
return 0;
}
@@ -707,6 +721,9 @@
if (self->tokenized_fields) {
hs_destroy(self->tokenized_fields);
}
+ if (self->dynbuf) {
+ free(self->dynbuf);
+ }
hs_destroy(self->all_fields);
hs_destroy(self->fields_buf);
h_destroy(self->field_cache);
@@ -754,6 +771,7 @@
self->analyzer = analyzer;
self->ts_cache = h_new_str(&free, (free_ft)&ts_deref);
self->buf_index = 0;
+ self->dynbuf = NULL;
self->non_tokenizer = non_tokenizer_new();
mutex_init(&self->mutex, NULL);
return self;
--
William <wmorgan-ferret at masanjin.net>
On 9/25/06, William Morgan <wmorgan-ferret at masanjin.net> wrote:> Hi Dave, > > The patch below corrects a buffer overflow bug in q_parser.y. Since it > is triggered by excessively long query strings, I believe that this bug > could be exploited to allow arbitrary code execution if a query string > supplied by a user is passed in directly to Ferret and not truncatated. > If I''m right, you should consider a new release asap. > > I''ve fixed it to simply allocate more memory if the standard buffers > aren''t enough (because I had some long (i.e. > 255) query strings that I > needed to support), but there are other solutions as well. > > Index: c/include/search.h > ==================================================================> --- c/include/search.h (revision 615) > +++ c/include/search.h (working copy) > @@ -819,6 +819,7 @@ > char *qstr; > char *qstrp; > char buf[QP_CONC_WORDS][MAX_WORD_SIZE]; > + char *dynbuf; > int buf_index; > HashTable *field_cache; > HashSet *fields; > Index: c/src/q_parser.y > ==================================================================> --- c/src/q_parser.y (revision 615) > +++ c/src/q_parser.y (working copy) > @@ -173,6 +173,11 @@ > char *bufp = buf; > qp->buf_index = (qp->buf_index + 1) % QP_CONC_WORDS; > > + if (qp->dynbuf) { > + free(qp->dynbuf); > + qp->dynbuf = NULL; > + } > + > qp->qstrp--; /* need to back up one character */ > > while (!strchr(not_word, (c=*qp->qstrp++))) { > @@ -192,6 +197,15 @@ > default: > *bufp++ = c; > } > + /* we''ve exceeded the static buffer. switch to the dynamic > + one. */ > + > + if (!qp->dynbuf && ((bufp - buf) == MAX_WORD_SIZE)) { > + qp->dynbuf = calloc(strlen(qp->qstr) + 1, sizeof(char)); > + strncpy(qp->dynbuf, buf, MAX_WORD_SIZE); > + buf = qp->dynbuf; > + bufp = buf + MAX_WORD_SIZE; > + } > } > qp->qstrp--; > /* check for keywords. There are only four so we have a bit of a hack which > @@ -262,7 +276,7 @@ > } > mutex_unlock(&qp->mutex); > RAISE(PARSE_ERROR, "couldn''t parse query ``%s''''. Error message " > - " was %se", buf, (char *)msg); > + "was: %s", buf, (char *)msg); > } > return 0; > } > @@ -707,6 +721,9 @@ > if (self->tokenized_fields) { > hs_destroy(self->tokenized_fields); > } > + if (self->dynbuf) { > + free(self->dynbuf); > + } > hs_destroy(self->all_fields); > hs_destroy(self->fields_buf); > h_destroy(self->field_cache); > @@ -754,6 +771,7 @@ > self->analyzer = analyzer; > self->ts_cache = h_new_str(&free, (free_ft)&ts_deref); > self->buf_index = 0; > + self->dynbuf = NULL; > self->non_tokenizer = non_tokenizer_new(); > mutex_init(&self->mutex, NULL); > return self; >Thanks, I''ll release a new gem ASAP. cheers, Dave