[PATCH] search: forbid getopt(3) switch injection in query

[PATCH] search: forbid getopt(3) switch injection in query

[Eric Wong]

Search queries may start with `-', confusing getopt(3) and
Getopt::Long; so we use `--' to separate the query string
from switches.

Fortunately, this doesn't allow writes to on-disk Xapian DBs,
but causes aborts on some searches or nonsensical results when
using the optional external xap_helper processes.  There's no
risk of data leaks since the mset xap_helper endpoint only
returns document IDs (unsigned integers), and not terms.

The biggest danger is may run systems out of space if the system
is configured to write out core dumps.
---
 lib/PublicInbox/Search.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index 25ef49c5..eb5e67ba 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -480,7 +480,7 @@ sub async_mset {
 	my ($self, $qry_str, $opt, $cb, @args) = @_;
 	if ($XHC) { # unconditionally retrieving pct + rank for now
 		xdb($self); # populate {nshards}
-		my @margs = ($self->xh_args, xh_opt($self, $opt));
+		my @margs = ($self->xh_args, xh_opt($self, $opt), '--');
 		my $ret = eval {
 			my $rd = $XHC->mkreq(undef, 'mset', @margs, $qry_str);
 			PublicInbox::XhcMset->maybe_new($rd, $self, $cb, @args);

Re: [PATCH] search: forbid getopt(3) switch injection in query

[Eric Wong]

Pushed with updated commit message:

1:  39ee8b08 ! 1:  3b7427bf search: forbid getopt(3) switch injection in query
    @@ Commit message
         Getopt::Long; so we use `--' to separate the query string
         from switches.
     
    -    Fortunately, this doesn't allow writes to on-disk Xapian DBs,
    -    but causes aborts on some searches or nonsensical results when
    -    using the optional external xap_helper processes.  There's no
    -    risk of data leaks since the mset xap_helper endpoint only
    -    returns document IDs (unsigned integers), and not terms.
    +    Consequences of this bug were limited to a single broken HTTP
    +    response for the requesting client.
     
    -    The biggest danger is may run systems out of space if the system
    -    is configured to write out core dumps.
    +    It didn't didn't allow writes to on-disk Xapian DBs, but caused
    +    aborts on some searches or nonsensical results when using the
    +    optional external xap_helper processes.  There was no risk of
    +    data leaks since the mset xap_helper endpoint only returns
    +    document IDs (unsigned integers), and not terms.
    +
    +    The biggest danger from this bug was that it could run systems
    +    out of space if they are configured to write out core dumps.
     
      ## lib/PublicInbox/Search.pm ##
     @@ lib/PublicInbox/Search.pm: sub async_mset {