imapserver: prevent unbounded memory allocations when handling a command

some commands, like search, can specify any number of literals, of arbitrary
size.  we already limited individual literals to 100kb. but you could specify
many of them, causing unbounded memory consumption. this change adds a limit of
1000 literals in a command, and a limit of 1mb of total combined memory for
literals. once the limits are exceeded, a TOOBIG error code is returned.

unbounded memory use could only be triggered on authenticated connections.

this addresses the same issue as CVE-2024-34055 for cyrus-imap, by damian
poddebniak.
This commit is contained in:
Mechiel Lukkien 2024-06-10 14:49:10 +02:00
parent 614576e409
commit aef99a72d8
No known key found for this signature in database
4 changed files with 105 additions and 23 deletions

View file

@ -238,19 +238,27 @@ func (c *Conn) Write(buf []byte) (n int, rerr error) {
// WriteSyncLiteral first writes the synchronous literal size, then read the // WriteSyncLiteral first writes the synchronous literal size, then read the
// continuation "+" and finally writes the data. // 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) defer c.recover(&rerr)
_, err := fmt.Fprintf(c.conn, "{%d}\r\n", len(s)) _, err := fmt.Fprintf(c.conn, "{%d}\r\n", len(s))
c.xcheckf(err, "write sync literal size") c.xcheckf(err, "write sync literal size")
line, err := c.Readline()
c.xcheckf(err, "read line") plus, err := c.r.Peek(1)
if !strings.HasPrefix(line, "+") { c.xcheckf(err, "read continuation")
c.xerrorf("no continuation received for sync literal") 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)) untagged, result, err := c.Response()
c.xcheckf(err, "write literal data") if err == nil && result.Status == OK {
return nil 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 // Transactf writes format and args as an IMAP command, using Commandf with an

View file

@ -48,11 +48,13 @@ type parser struct {
// Orig is the line in original casing, and upper in upper casing. We often match // 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 // against upper for easy case insensitive handling as IMAP requires, but sometimes
// return from orig to keep the original case. // return from orig to keep the original case.
orig string orig string
upper string upper string
o int // Current offset in parsing. o int // Current offset in parsing.
contexts []string // What we're parsing, for error messages. contexts []string // What we're parsing, for error messages.
conn *conn 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 // 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 { 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) { 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") 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) s := p.conn.xreadliteral(size, sync)
line := p.conn.readline(false) line := p.conn.readline(false)
p.orig, p.upper, p.o = line, toUpper(line), 0 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 // ../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? // todo: enforce that we get non-binary when ~ isn't present?
if lit8 { if lit8 {
p.take("~") p.take("~")
} }
p.xtake("{") p.xtake("{")
size = p.xnumber64() 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("+") sync = !p.take("+")
p.xtake("}") p.xtake("}")
p.xempty() 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 return size, sync
} }

View file

@ -1,6 +1,7 @@
package imapserver package imapserver
import ( import (
"fmt"
"strconv" "strconv"
"strings" "strings"
"testing" "testing"
@ -338,6 +339,53 @@ func TestSearch(t *testing.T) {
tc.client.Enable("IMAP4rev2") tc.client.Enable("IMAP4rev2")
tc.transactf("ok", `search undraft`) tc.transactf("ok", `search undraft`)
tc.xesearch(esearchall("1:2")) 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. // esearchall makes an UntaggedEsearch response with All set, for comparisons.

View file

@ -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? // todo: this is only relevant if we also support the CATENATE extension?
// ../rfc/6855:204 // ../rfc/6855:204
utf8 := p.take("UTF8 (") utf8 := p.take("UTF8 (")
size, sync := p.xliteralSize(0, utf8) size, sync := p.xliteralSize(utf8, false)
name = xcheckmailboxname(name, true) name = xcheckmailboxname(name, true)
c.xdbread(func(tx *bstore.Tx) { c.xdbread(func(tx *bstore.Tx) {