upas/fs/imap.c: additional sanity checking during `Expunge` to avoid suicide

We have to ensure the size we compute for memmove(...) in `Expunge` is
properly bounded.  For certain combinations of inputs we compute an
illegal size causing a suicide:

	upas/fs: imap: fetchrsp: fetchrsp: bad idx 7
	upas/fs: user: igor; note: sys: trap: fault read addr=0x0 pc=0x214f92
	fs 531: suicide: sys: trap: fault read addr=0x0 pc=0x214f92

Stack trace including state of data-structures:

	term% acid -l /sys/src/cmd/upas/fs/imap.acid 531
	/proc/531/text:amd64 plan 9 executable
	/sys/lib/acid/port
	/sys/lib/acid/amd64
	acid: lstk()
	memmove(p2=0x8e5928,n=0xffffffffffffffe8)+0x42 /sys/src/libc/amd64/memmove.s:36
	imap4resp0(imap=0x423c80,mb=0x0,m=0x0)+0x448 /sys/src/cmd/upas/fs/imap.c:522
		unexp=0x58b00000000
		p=0x425d17
		ep=0x425d17
		line=0x425d0b
		n=0x425d100000058c
		verb=0x425d10
		op=0x2e6d6f63
	imap4resp()+0x1b /sys/src/cmd/upas/fs/imap.c:556
	imap4read(imap=0x423c80,mb=0x4239e0)+0x30 /sys/src/cmd/upas/fs/imap.c:928
		s=0x425d29
		f=0x425d29
		n=0x58b0000058c
		ll=0x5008a34b0
		i=0x425d290000058b
		m=0x171bd1f85150e2a6
	imap4sync(mb=0x4239e0)+0x62 /sys/src/cmd/upas/fs/imap.c:1059
		imap=0x423c80
		err=0x0
	syncmbox(mb=0x4239e0,doplumb=0x70616d6900000001)+0x5c /sys/src/cmd/upas/fs/mbox.c:76
		a=0x58f
		n=0x400bc800000000
		d=0x0
		y=0x0
		next=0x171bd206499a9366
		m=0x66939a4906d21b17
	reader()+0xde /sys/src/cmd/upas/fs/fs.c:1488
		t=0x204ca163404153
		mb=0x692e6d68656f622f
	io()+0x212 /sys/src/cmd/upas/fs/fs.c:1416
		n=0x0
	main(argc=0x0,argv=0x7fffffffef58)+0x31e /sys/src/cmd/upas/fs/fs.c:353
		mboxfile=0x7fffffffef7a
		nodflt=0x300000000
		srvpost=0x0
		v=0x7fffffffef38
		_argc=0x6d
		_args=0x40d7bd
		p=0x400000003
		maildir=0x0
		mbox=0x0
		srvfile=0x0
	_main+0x40 /sys/src/libc/amd64/main9.s:15
	acid: Imap(0x423c80)
		mbox	0x0000000000428b90
		freep	0x0000000000423c20
		host	0x0000000000423c27
		user	0x0000000000423c36
		refreshtime	60
		cap	0x00
		flags	0x05
		tag	872
		validity	605040277
		newvalidity	605040277
		nmsg	1419
		size	0
		f	0x00000000008dd408
		nuid	1420
		muid	1420
	Biobuf bin {
	Biobufhdr {
		icount	-28
		ocount	0
		rdline	16
		runesize	0
		state	1
		fid	10
		flag	0
		offset	280380
		bsize	8192
		bbuf	0x0000000000423d35
		ebuf	0x0000000000425d35
		gbuf	0x0000000000425cd1
		errorf	0x0000000000000000
		iof	0x0000000000228d17
		aux	0x0000000000000000
	}
		b	end+0x388
	}
	Biobuf bout {
	Biobufhdr {
		icount	0
		ocount	-8192
		rdline	0
		runesize	0
		state	2
		fid	10
		flag	0
		offset	33362
		bsize	8192
		bbuf	0x0000000000425d9d
		ebuf	0x0000000000427d9d
		gbuf	0x0000000000427d9d
		errorf	0x0000000000000000
		iof	0x0000000000228d3a
		aux	0x0000000000000000
	}
		b	0x425d98
	}
		binit	1
		fd	10

The root cause is an integer underflow where we subtract -1 from 0
using an unsigned type.

The above acid trace shows the following values for key local variables:

• nmsg ... 1419
• muid ... 1420
• n    ... 1420
• idx  ... 1419

The key section of code is /sys/src/cmd/upas/fs/imap.c:516,522

				case Expunge:
					if(n < 1 || n > imap->muid){
						snprint(error, sizeof(error), "bad expunge %d (nmsg %d)", n, imap->nuid);
						return error;
					}
					idx = n - 1;
					memmove(&imap->f[idx], &imap->f[idx + 1], (imap->nmsg - idx - 1)*sizeof(imap->f[0]));

Plugging the above values into the call to memmove(...) demonstrates the
issue:

	memmove(&imap->f[1419], &imap->f[1419 + 1], (1419 - 1419 - 1)*sizeof(imap->f[0]));
	                                             ^^^^^^^^^^^^^^^

The third argument of memmove(...) is an unsigned size type causing
the size to be a value that is way too large (the stack trace shows
the size to be the unsigned value 0xffffffffffffffe8).

The `Expunge` case can be fixed by amending the bounds checking
condition before the memmove(...):

			case Expunge:
				if(n < 1 || n > imap->muid || (n - 1) >= imap->nmsg){
                                              ^^^^^^^^^^^^^^^^^^^^^
					snprint(error, sizeof(error), "bad expunge %d (nmsg %d)", n, imap->nuid);
					return error;
				}
				idx = n - 1;
				memmove(&imap->f[idx], &imap->f[idx + 1], (imap->nmsg - idx - 1)*sizeof(imap->f[0]));

To add some additional context, this issue has been introduced in
revision:

	term% git/export 84c4c81ceecfa8f51949787fc2dbe7b14164a353
	Date: Mon, 02 Dec 2019 01:12:19 +0000
	Subject: [PATCH] upas/fs imap fixes and improvements

	do incremental imap fetches after startup, fixes validity handling,
	record flags correctly when we aren't in the process of directly
	updating a message, fixes off by one in flag parsing, fixes
	mis-indexing messages in sync when we get an unsolicited fetch
	response.
    ...

1 files changed, 1 insertions, 1 deletions

diff --git a/sys/src/cmd/upas/fs/imap.c b/sys/src/cmd/upas/fs/imap.c
index 187b1ace9..938e97ce6 100644
--- a/sys/src/cmd/upas/fs/imap.c
+++ b/sys/src/cmd/upas/fs/imap.c
@@ -514,7 +514,7 @@ imap4resp0(Imap *imap, Mailbox *mb, Message *m)
 					imap->nuid = n;
 				break;
 			case Expunge:
-				if(n < 1 || n > imap->muid){
+				if(n < 1 || n > imap->muid || (n - 1) >= imap->nmsg){
 					snprint(error, sizeof(error), "bad expunge %d (nmsg %d)", n, imap->nuid);
 					return error;
 				}