On Tue, 22 Sep 2009, Don Armstrong wrote:
> On Tue, 22 Sep 2009, Timo Sirainen wrote:
> > On Sep 22, 2009, at 9:46 AM, Don Armstrong wrote:
> >
> > >Attached please find the trivial patch for this security fix.
> >
> > snprintf, not sprintf:
> >
> > - sprintf(errbuf, "flag '%s': not a valid relational operation", r);
> > + sprintf(errbuf, sizeof(errbuf), "flag '%s': not a valid
> > relational operation", r);
> >
>
> Eek. Yes, right. I'll attach a corrected patch one I rebuild
> everything again. (Or DSA can continue on with this trivial fix).
Please find attached patches for etch and lenny which should resolve
this issue.
Don Armstrong
--
The beauty of the DRUNKENNESS subprogram was that you could move your
intoxication level up and down at will, instead of being caught on a
relentless down escalator to bargain basement philosophy and the
parking garage.
-- Rudy von Bitter _Software_ p124
http://www.donarmstrong.com http://rzlab.ucr.edu
diff -u dovecot-1.0.rc15/dovecot-sieve/src/libsieve/bc_eval.c dovecot-1.0.rc15/dovecot-sieve/src/libsieve/bc_eval.c
--- dovecot-1.0.rc15/dovecot-sieve/src/libsieve/bc_eval.c
+++ dovecot-1.0.rc15/dovecot-sieve/src/libsieve/bc_eval.c
@@ -440,7 +440,7 @@
int comparator=ntohl(bc[i+3].value);
int apart=ntohl(bc[i+4].value);
int count=0;
- char scount[3];
+ char scount[20];
int isReg = (match==B_REGEX);
int ctag = 0;
regex_t *reg;
@@ -574,7 +574,7 @@
if (match == B_COUNT)
{
- sprintf(scount, "%u", count);
+ snprintf(scount, sizeof(scount), "%u", count);
/* search through all the data */
currd=datai+2;
for (z=0; z<numdata && !res; z++)
@@ -608,7 +608,7 @@
int relation=ntohl(bc[i+2].value);
int comparator=ntohl(bc[i+3].value);
int count=0;
- char scount[3];
+ char scount[20];
int isReg = (match==B_REGEX);
int ctag = 0;
regex_t *reg;
@@ -689,7 +689,7 @@
if (match == B_COUNT )
{
- sprintf(scount, "%u", count);
+ snprintf(scount, sizeof(scount), "%u", count);
/*search through all the data*/
currd=datai+2;
for (z=0; z<numdata && !res; z++)
diff -u dovecot-1.0.rc15/dovecot-sieve/src/libsieve/script.c dovecot-1.0.rc15/dovecot-sieve/src/libsieve/script.c
--- dovecot-1.0.rc15/dovecot-sieve/src/libsieve/script.c
+++ dovecot-1.0.rc15/dovecot-sieve/src/libsieve/script.c
@@ -526,9 +526,9 @@
if ((ret != SIEVE_OK) && interp->err) {
char buf[1024];
if (lastaction == -1) /* we never executed an action */
- sprintf(buf, "%s", errmsg ? errmsg : sieve_errstr(ret));
+ snprintf(buf, sizeof(buf), "%s", errmsg ? errmsg : sieve_errstr(ret));
else
- sprintf(buf, "%s: %s", action_to_string(lastaction),
+ snprintf(buf, sizeof(buf), "%s: %s", action_to_string(lastaction),
errmsg ? errmsg : sieve_errstr(ret));
ret |= interp->execute_err(buf, interp->interp_context,
@@ -546,7 +546,7 @@
ret |= keep_ret;
if (keep_ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Kept\n");
else {
implicit_keep = 0; /* don't try an implicit keep again */
@@ -599,7 +599,7 @@
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Rejected with: %s\n", a->u.rej.msg);
break;
@@ -615,7 +615,7 @@
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Filed into: %s\n",a->u.fil.mailbox);
break;
case ACTION_KEEP:
@@ -629,7 +629,7 @@
&errmsg);
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Kept\n");
break;
case ACTION_REDIRECT:
@@ -643,7 +643,7 @@
&errmsg);
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Redirected to %s\n", a->u.red.addr);
break;
case ACTION_DISCARD:
@@ -655,7 +655,7 @@
&errmsg);
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Discarded\n");
break;
@@ -689,12 +689,12 @@
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Sent vacation reply\n");
} else if (ret == SIEVE_DONE) {
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Vacation reply suppressed\n");
ret = SIEVE_OK;
diff -u dovecot-1.0.rc15/dovecot-sieve/src/libsieve/sieve.y dovecot-1.0.rc15/dovecot-sieve/src/libsieve/sieve.y
--- dovecot-1.0.rc15/dovecot-sieve/src/libsieve/sieve.y
+++ dovecot-1.0.rc15/dovecot-sieve/src/libsieve/sieve.y
@@ -922,7 +922,7 @@
else if (!strcmp(r, "ne")) {return NE;}
else if (!strcmp(r, "eq")) {return EQ;}
else{
- sprintf(errbuf, "flag '%s': not a valid relational operation", r);
+ snprintf(errbuf, sizeof(errbuf), "flag '%s': not a valid relational operation", r);
yyerror(errbuf);
return -1;
}
diff -u dovecot-1.0.rc15/debian/changelog dovecot-1.0.rc15/debian/changelog
--- dovecot-1.0.rc15/debian/changelog
+++ dovecot-1.0.rc15/debian/changelog
@@ -1,3 +1,11 @@
+dovecot (1.0.rc15-2etch5) stable-security; urgency=high
+
+ * Non-maintainer upload by the security team.
+ * Fix for bufffer overflow in SIEVE filtering allowing for privilege
+ escalation (closes: #546656)
+
+ -- Don Armstrong <[email protected]> Tue, 22 Sep 2009 11:31:47 -0700
+
dovecot (1.0.rc15-2etch4) stable-security; urgency=high
* Security issue: some passdbs allowed users to log in without a valid
diff -u dovecot-1.0.15/dovecot-sieve/src/libsieve/script.c dovecot-1.0.15/dovecot-sieve/src/libsieve/script.c
--- dovecot-1.0.15/dovecot-sieve/src/libsieve/script.c
+++ dovecot-1.0.15/dovecot-sieve/src/libsieve/script.c
@@ -526,9 +526,9 @@
if ((ret != SIEVE_OK) && interp->err) {
char buf[1024];
if (lastaction == -1) /* we never executed an action */
- sprintf(buf, "%s", errmsg ? errmsg : sieve_errstr(ret));
+ snprintf(buf, sizeof(buf), "%s", errmsg ? errmsg : sieve_errstr(ret));
else
- sprintf(buf, "%s: %s", action_to_string(lastaction),
+ snprintf(buf, sizeof(buf), "%s: %s", action_to_string(lastaction),
errmsg ? errmsg : sieve_errstr(ret));
ret |= interp->execute_err(buf, interp->interp_context,
@@ -546,7 +546,7 @@
ret |= keep_ret;
if (keep_ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Kept\n");
else {
implicit_keep = 0; /* don't try an implicit keep again */
@@ -599,7 +599,7 @@
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Rejected with: %s\n", a->u.rej.msg);
break;
@@ -615,7 +615,7 @@
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Filed into: %s\n",a->u.fil.mailbox);
break;
case ACTION_KEEP:
@@ -629,7 +629,7 @@
&errmsg);
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Kept\n");
break;
case ACTION_REDIRECT:
@@ -643,7 +643,7 @@
&errmsg);
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Redirected to %s\n", a->u.red.addr);
break;
case ACTION_DISCARD:
@@ -655,7 +655,7 @@
&errmsg);
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Discarded\n");
break;
@@ -689,12 +689,12 @@
if (ret == SIEVE_OK)
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Sent vacation reply\n");
} else if (ret == SIEVE_DONE) {
snprintf(actions_string+strlen(actions_string),
- sizeof(actions_string)-strlen(actions_string),
+ ACTIONS_STRING_LEN-strlen(actions_string),
"Vacation reply suppressed\n");
ret = SIEVE_OK;
diff -u dovecot-1.0.15/dovecot-sieve/src/libsieve/sieve.y dovecot-1.0.15/dovecot-sieve/src/libsieve/sieve.y
--- dovecot-1.0.15/dovecot-sieve/src/libsieve/sieve.y
+++ dovecot-1.0.15/dovecot-sieve/src/libsieve/sieve.y
@@ -922,7 +922,7 @@
else if (!strcmp(r, "ne")) {return NE;}
else if (!strcmp(r, "eq")) {return EQ;}
else{
- sprintf(errbuf, "flag '%s': not a valid relational operation", r);
+ snprintf(errbuf, sizeof(errbuf), "flag '%s': not a valid relational operation", r);
yyerror(errbuf);
return -1;
}
diff -u dovecot-1.0.15/dovecot-sieve/src/libsieve/bc_eval.c dovecot-1.0.15/dovecot-sieve/src/libsieve/bc_eval.c
--- dovecot-1.0.15/dovecot-sieve/src/libsieve/bc_eval.c
+++ dovecot-1.0.15/dovecot-sieve/src/libsieve/bc_eval.c
@@ -475,7 +475,7 @@
int comparator=ntohl(bc[i+3].value);
int apart=ntohl(bc[i+4].value);
int count=0;
- char scount[3];
+ char scount[20];
int isReg = (match==B_REGEX);
int ctag = 0;
regex_t *reg;
@@ -609,7 +609,7 @@
if (match == B_COUNT)
{
- sprintf(scount, "%u", count);
+ snprintf(scount, sizeof(scount), "%u", count);
/* search through all the data */
currd=datai+2;
for (z=0; z<numdata && !res; z++)
@@ -643,7 +643,7 @@
int relation=ntohl(bc[i+2].value);
int comparator=ntohl(bc[i+3].value);
int count=0;
- char scount[3];
+ char scount[20];
int isReg = (match==B_REGEX);
int ctag = 0;
regex_t *reg;
@@ -724,7 +724,7 @@
if (match == B_COUNT )
{
- sprintf(scount, "%u", count);
+ snprintf(scount, sizeof(scount), "%u", count);
/*search through all the data*/
currd=datai+2;
for (z=0; z<numdata && !res; z++)
diff -u dovecot-1.0.15/debian/changelog dovecot-1.0.15/debian/changelog
--- dovecot-1.0.15/debian/changelog
+++ dovecot-1.0.15/debian/changelog
@@ -1,3 +1,11 @@
+dovecot (1:1.0.15-2.3+lenny1) unstable; urgency=low
+
+ * Non-maintainer upload by the security team.
+ * Fix for buffer overflow in SIEVE filtering allowing for privilege
+ escalation (closes: #546656)
+
+ -- Don Armstrong <[email protected]> Mon, 21 Sep 2009 21:05:52 -0700
+
dovecot (1:1.0.15-2.3) unstable; urgency=medium
* Non-maintainer upload