On Wed, Jun 16, 2010 at 11:49:22AM -0500, Patrick Goetz wrote:
> On 06/15/2010 06:36 PM, Bron Gondwana wrote:
> >>
> >>~/imap/imapparse.c
> >>line 53:
> >>- MAXLITERAL = INT_MAX / 20
> >>+ MAXLITERAL = INT_MAX / 10
> >
> >Hmm - and still untouched in my latest codebase too. We've got a
> >configurable maximum line length now.
> >
> >That's basically what - 100M, changed up to 200M? Or am I out by an order
> >of magnitude or so? Maximum size for a single email.
> >
> >I'd be happy to apply it (and also to replication...) - I don't see the
> >danger with either option, but it will block super-large emails.
> >
>
> Wouldn't the len*10 here:
> len = len*10 + c - '0';
> if (len > MAXLITERAL || len < 0) {
>
> imply that it's actually changing a 10.7MB max size to 21.4MB?
No. That's a skanky integer parser. "len" is the integer value of
the number that's been read so far, the *10 was the previously read
character. I guess that to avoid overflow, you need to limit what
you can read here... hmm. I think that function could do with a
rewrite actually.
> In any case, since most people use email as a defacto file sharing
> system, this should be a run-time option, if possible. On my
> postfix server at work we had message_size_limit set to 50MB (or
> something like this) and my own spouse got burned by this when some
> consultant tried to email her a large attachment containing some
> work-related videos. I got the "so and so says he's trying to send
> me some files and the message keeps bouncing" call, which is how I
> discovered the message_size_limit setting in main.cf in the first
> place.
>
> So, eventually MAXLITERAL should also be configurable, if possible,
> but for now we can probably let the debian *2 size limit patch
> stand?
I've attached the patch I just put into future branch. It makes it
configurable. But the more I think about it, the more I think the
whole concept is actually broken. I'm probably going to just remove
the limit and change how we read the number!
Bron.
diff --git a/imap/imapparse.c b/imap/imapparse.c
index 3e1ec54..43a00e6 100644
--- a/imap/imapparse.c
+++ b/imap/imapparse.c
@@ -52,10 +52,6 @@
#include "global.h"
#include "exitcodes.h"
-enum {
- MAXLITERAL = INT_MAX / 20
-};
-
void freebuf(struct buf *buf)
{
if (buf->s) {
@@ -199,10 +195,10 @@ int getxstring(struct protstream *pin, struct protstream *pout,
while ((c = prot_getc(pin)) != EOF && isdigit(c)) {
sawdigit = 1;
len = len*10 + c - '0';
- if (len > MAXLITERAL || len < 0) {
- /* we overflowed */
- fatal("literal too big", EC_IOERR);
- }
+ if (len > config_maxliteral || len < 0) {
+ /* we overflowed */
+ fatal("literal too big", EC_IOERR);
+ }
}
if (c == '+') {
isnowait++;
diff --git a/imap/sync_support.c b/imap/sync_support.c
index ca4934d..23272d0 100644
--- a/imap/sync_support.c
+++ b/imap/sync_support.c
@@ -91,10 +91,6 @@
const char *lastkey = NULL;
-enum {
- MAXLITERAL = INT_MAX / 20
-};
-
char *sync_encode_options(int options)
{
static char buf[4];
diff --git a/lib/imapoptions b/lib/imapoptions
index 5215ec2..cd7a494 100644
--- a/lib/imapoptions
+++ b/lib/imapoptions
@@ -617,6 +617,11 @@ Blank lines and lines beginning with ``#'' are ignored.
the lines in the header will be skipped. This is to avoid malformed
messages causing giant cache records */
+{ "maxliteral", INT_MAX / 10, INT }
+/* Maximum size of a literal value. This sets the maximum possible size
+ for an email. The default is INT_MAX/10 or about 210Mb on a standard
+ 32 bit system */
+
{ "maxmessagesize", 0, INT }
/* Maximum incoming LMTP message size. If non-zero, lmtpd will reject
messages larger than \fImaxmessagesize\fR bytes. If set to 0, this
diff --git a/lib/libconfig.c b/lib/libconfig.c
index 1c59ce6..ccea820 100644
--- a/lib/libconfig.c
+++ b/lib/libconfig.c
@@ -81,6 +81,7 @@ int config_hashimapspool; /* f */
enum enum_value config_virtdomains; /* f */
enum enum_value config_mupdate_config; /* IMAP_ENUM_MUPDATE_CONFIG_STANDARD */
int config_auditlog;
+int config_maxliteral;
int config_maxword;
int config_maxquoted;
@@ -330,6 +331,7 @@ void config_read(const char *alt_config)
}
/* set some limits */
+ config_maxliteral = config_getint(IMAPOPT_MAXLITERAL);
config_maxquoted = config_getint(IMAPOPT_MAXQUOTED);
config_maxword = config_getint(IMAPOPT_MAXWORD);
}
diff --git a/lib/libconfig.h b/lib/libconfig.h
index de40784..d70ce31 100644
--- a/lib/libconfig.h
+++ b/lib/libconfig.h
@@ -79,6 +79,7 @@ extern int config_implicitrights;
extern enum enum_value config_virtdomains;
extern enum enum_value config_mupdate_config;
extern int config_auditlog;
+extern int config_maxliteral;
extern int config_maxquoted;
extern int config_maxword;
diff --git a/tools/config2header b/tools/config2header
index 97ebec3..50a4085 100755
--- a/tools/config2header
+++ b/tools/config2header
@@ -125,6 +125,7 @@ print CFILE <<EOF
#include <syslog.h>
#include <stdlib.h>
#include <string.h>
+#include <limits.h>
#include "imapopts.h"
struct imapopt_s imapopts[] =
