On 9/25/06, William Morgan <[EMAIL PROTECTED]> 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
_______________________________________________
Ferret-talk mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/ferret-talk

Reply via email to