diff --git a/imapclient/client.go b/imapclient/client.go index c9addb533c..6f45fda2fe 100644 --- a/imapclient/client.go +++ b/imapclient/client.go @@ -238,19 +238,27 @@ func (c *Conn) Write(buf []byte) (n int, rerr error) { // WriteSyncLiteral first writes the synchronous literal size, then read the // continuation "+" and finally writes the data. -func (c *Conn) WriteSyncLiteral(s string) (rerr error) { +func (c *Conn) WriteSyncLiteral(s string) (untagged []Untagged, rerr error) { defer c.recover(&rerr) _, err := fmt.Fprintf(c.conn, "{%d}\r\n", len(s)) c.xcheckf(err, "write sync literal size") - line, err := c.Readline() - c.xcheckf(err, "read line") - if !strings.HasPrefix(line, "+") { - c.xerrorf("no continuation received for sync literal") + + plus, err := c.r.Peek(1) + c.xcheckf(err, "read continuation") + if plus[0] == '+' { + _, err = c.Readline() + c.xcheckf(err, "read continuation line") + + _, err = c.conn.Write([]byte(s)) + c.xcheckf(err, "write literal data") + return nil, nil } - _, err = c.conn.Write([]byte(s)) - c.xcheckf(err, "write literal data") - return nil + untagged, result, err := c.Response() + if err == nil && result.Status == OK { + c.xerrorf("no continuation, but invalid ok response (%q)", result.More) + } + return untagged, fmt.Errorf("no continuation (%s)", result.Status) } // Transactf writes format and args as an IMAP command, using Commandf with an diff --git a/imapserver/parse.go b/imapserver/parse.go index c03865966e..22a2877e59 100644 --- a/imapserver/parse.go +++ b/imapserver/parse.go @@ -48,11 +48,13 @@ type parser struct { // Orig is the line in original casing, and upper in upper casing. We often match // against upper for easy case insensitive handling as IMAP requires, but sometimes // return from orig to keep the original case. - orig string - upper string - o int // Current offset in parsing. - contexts []string // What we're parsing, for error messages. - conn *conn + orig string + upper string + o int // Current offset in parsing. + contexts []string // What we're parsing, for error messages. + literals int // Literals in command, for limit. + literalSize int64 // Total size of literals in command, for limit. + conn *conn } // toUpper upper cases bytes that are a-z. strings.ToUpper does too much. and @@ -70,7 +72,7 @@ func toUpper(s string) string { } func newParser(s string, conn *conn) *parser { - return &parser{s, toUpper(s), 0, nil, conn} + return &parser{s, toUpper(s), 0, nil, 0, 0, conn} } func (p *parser) xerrorf(format string, args ...any) { @@ -302,7 +304,7 @@ func (p *parser) xstring() (r string) { } p.xerrorf("missing closing dquote in string") } - size, sync := p.xliteralSize(100*1024, false) + size, sync := p.xliteralSize(false, true) s := p.conn.xreadliteral(size, sync) line := p.conn.readline(false) p.orig, p.upper, p.o = line, toUpper(line), 0 @@ -741,23 +743,47 @@ func (p *parser) xdateTime() time.Time { } // ../rfc/9051:6655 ../rfc/7888:330 ../rfc/3501:4801 -func (p *parser) xliteralSize(maxSize int64, lit8 bool) (size int64, sync bool) { +func (p *parser) xliteralSize(lit8 bool, checkSize bool) (size int64, sync bool) { // todo: enforce that we get non-binary when ~ isn't present? if lit8 { p.take("~") } p.xtake("{") size = p.xnumber64() - if maxSize > 0 && size > maxSize { - // ../rfc/7888:249 - line := fmt.Sprintf("* BYE [ALERT] Max literal size %d is larger than allowed %d in this context", size, maxSize) - err := errors.New("literal too big") - panic(syntaxError{line, "TOOBIG", err.Error(), err}) - } sync = !p.take("+") p.xtake("}") p.xempty() + + if checkSize { + // ../rfc/7888:249 + var errmsg string + const ( + litSizeMax = 100 * 1024 + totalLitSizeMax = 10 * litSizeMax + litMax = 1000 + ) + p.literalSize += size + p.literals++ + if size > litSizeMax { + errmsg = fmt.Sprintf("max literal size %d is larger than allowed %d", size, litSizeMax) + } else if p.literalSize > totalLitSizeMax { + errmsg = fmt.Sprintf("max total literal size for command %d is larger than allowed %d", p.literalSize, totalLitSizeMax) + } else if p.literals > litMax { + errmsg = fmt.Sprintf("max literals for command %d is larger than allowed %d", p.literals, litMax) + } + if errmsg != "" { + // ../rfc/9051:357 ../rfc/3501:347 + err := errors.New("literal too big: " + errmsg) + if sync { + errmsg = "" + } else { + errmsg = "* BYE [ALERT] " + errmsg + } + panic(syntaxError{errmsg, "TOOBIG", err.Error(), err}) + } + } + return size, sync } diff --git a/imapserver/search_test.go b/imapserver/search_test.go index 304f55b77c..80059a3342 100644 --- a/imapserver/search_test.go +++ b/imapserver/search_test.go @@ -1,6 +1,7 @@ package imapserver import ( + "fmt" "strconv" "strings" "testing" @@ -338,6 +339,53 @@ func TestSearch(t *testing.T) { tc.client.Enable("IMAP4rev2") tc.transactf("ok", `search undraft`) tc.xesearch(esearchall("1:2")) + + // Long commands should be rejected, not allocating too much memory. + lit := make([]byte, 100*1024+1) + for i := range lit { + lit[i] = 'x' + } + writeTextLit := func(n int, expok bool) { + _, err := fmt.Fprintf(tc.client, " TEXT ") + tcheck(t, err, "write text") + + _, err = fmt.Fprintf(tc.client, "{%d}\r\n", n) + tcheck(t, err, "write literal size") + line, err := tc.client.Readline() + tcheck(t, err, "read line") + if expok && !strings.HasPrefix(line, "+") { + tcheck(t, fmt.Errorf("no continuation after writing size: %s", line), "sending literal") + } else if !expok && !strings.HasPrefix(line, "x0 BAD [TOOBIG]") { + tcheck(t, fmt.Errorf("got line %s", line), "expected TOOBIG error") + } + if !expok { + return + } + _, err = tc.client.Write(lit[:n]) + tcheck(t, err, "write literal data") + } + + // More than 100k for a literal. + _, err := fmt.Fprintf(tc.client, "x0 uid search") + tcheck(t, err, "write start of uit search") + writeTextLit(100*1024+1, false) + + // More than 1mb total for literals. + _, err = fmt.Fprintf(tc.client, "x0 uid search") + tcheck(t, err, "write start of uit search") + for i := 0; i < 10; i++ { + writeTextLit(100*1024, true) + } + writeTextLit(1, false) + + // More than 1000 literals. + _, err = fmt.Fprintf(tc.client, "x0 uid search") + tcheck(t, err, "write start of uit search") + for i := 0; i < 1000; i++ { + writeTextLit(1, true) + } + writeTextLit(1, false) + } // esearchall makes an UntaggedEsearch response with All set, for comparisons. diff --git a/imapserver/server.go b/imapserver/server.go index 82947d2edf..d120ec0da8 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -2729,7 +2729,7 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) { // todo: this is only relevant if we also support the CATENATE extension? // ../rfc/6855:204 utf8 := p.take("UTF8 (") - size, sync := p.xliteralSize(0, utf8) + size, sync := p.xliteralSize(utf8, false) name = xcheckmailboxname(name, true) c.xdbread(func(tx *bstore.Tx) {