From 142b2498bf1898695d1878174033f46fed7d6262 Mon Sep 17 00:00:00 2001
From: Mechiel Lukkien <mechiel@ueber.net>
Date: Wed, 28 Jun 2023 19:41:58 +0200
Subject: [PATCH] fix two parsing bugs in imapserver

these could cause the parser to reject correct commands.

the first bug is about the allowed chars for an "atom", we were accepting too
many. this probably isn't easily triggered in practice.

the second bug is about how numbers (digits) are parsed. when gathering digits
to parse as number, we didn't consider only the directly upcoming digits that
make up the number, but continued looking for digits later on in the command.
then we tried to parse a string that was too long as a number, which would fail
because of additional characters. this could have been triggered with commands
containing two numbers. this is possible with e.g. "tag search or larger 123
smaller 123", the "or" takes two search keys again, each with a number. not too
common, but can happen.

found while writing tests for upcoming condstore/qresync implementation.
---
 imapserver/parse.go       | 20 +++++++++++---------
 imapserver/search_test.go |  3 +++
 imapserver/server_test.go |  1 +
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/imapserver/parse.go b/imapserver/parse.go
index 74f5f09..c699f83 100644
--- a/imapserver/parse.go
+++ b/imapserver/parse.go
@@ -12,12 +12,13 @@ import (
 )
 
 var (
-	listWildcards = "%*"
-	char          = charRange('\x01', '\x7f')
-	ctl           = charRange('\x01', '\x19')
-	atomChar      = charRemove(char, "(){ "+listWildcards+ctl)
-	respSpecials  = atomChar + "]"
-	astringChar   = atomChar + respSpecials
+	listWildcards  = "%*"
+	char           = charRange('\x01', '\x7f')
+	ctl            = charRange('\x01', '\x19')
+	quotedSpecials = `"\`
+	respSpecials   = "]"
+	atomChar       = charRemove(char, "(){ "+ctl+listWildcards+quotedSpecials+respSpecials)
+	astringChar    = atomChar + respSpecials
 )
 
 func charRange(first, last rune) string {
@@ -122,7 +123,7 @@ func (p *parser) take(s string) bool {
 
 func (p *parser) xtake(s string) {
 	if !p.take(s) {
-		p.xerrorf("expected %q", s)
+		p.xerrorf("expected %s", s)
 	}
 }
 
@@ -201,9 +202,10 @@ func (p *parser) xspace() {
 func (p *parser) digits() string {
 	var n int
 	for _, c := range p.upper[p.o:] {
-		if c >= '0' && c <= '9' {
-			n++
+		if c < '0' || c > '9' {
+			break
 		}
+		n++
 	}
 	if n == 0 {
 		return ""
diff --git a/imapserver/search_test.go b/imapserver/search_test.go
index 621bc6e..25b94b9 100644
--- a/imapserver/search_test.go
+++ b/imapserver/search_test.go
@@ -212,6 +212,9 @@ func TestSearch(t *testing.T) {
 	tc.transactf("ok", `search uid 5`)
 	tc.xsearch(1)
 
+	tc.transactf("ok", `search or larger 1000000 smaller 1`)
+	tc.xsearch()
+
 	tc.transactf("ok", `search undraft`)
 	tc.xsearch(1, 2)
 
diff --git a/imapserver/server_test.go b/imapserver/server_test.go
index fa4a718..f9c5849 100644
--- a/imapserver/server_test.go
+++ b/imapserver/server_test.go
@@ -191,6 +191,7 @@ func (tc *testconn) xcodeArg(v any) {
 }
 
 func (tc *testconn) xuntagged(exps ...any) {
+	tc.t.Helper()
 	tc.xuntaggedCheck(true, exps...)
 }