Re: mail/procmail CVE-2017-16844

2022-05-04 Thread Josh Grosse
On Thu, May 05, 2022 at 12:09:37AM +0100, Stuart Henderson wrote:

> It has been de-abandoned upstream, there is a new release from earlier this
> year. Update diff for that below FWIW. It builds, runtime not tested, I
> have forgotten how to use it.

I've just tested it on amd64, and it works fine with my narrow use case:
as an mda which routes to Maildirs, and has a simple SpamAssasin header
inspection for routing to the user's spam Maildir.



Re: mail/procmail CVE-2017-16844

2022-05-04 Thread Stuart Henderson
On 2022/05/04 23:42, Martin Schröder wrote:
> Am Mi., 6. Dez. 2017 um 13:06 Uhr schrieb Stuart Henderson
> :
> > OK for the fix. But guenther@'s comment from 2015 still stands -
> >
> > "Executive summary: delete the procmail port; the code is not safe and
> > should not be used as a basis for any further work."
> >
> > (https://marc.info/?l=openbsd-ports=141634350915839=2)
> 
> See also https://anarc.at/blog/2022-03-02-procmail-considered-harmful/
> 
> "TL;DR: procmail is a security liability and has been abandoned upstream
> for the last two decades. If you are still using it, you should probably drop
> everything and at least remove its SUID flag. There are plenty of alternatives
> to choose from, and conversion is a one-time, acceptable trade-off."
> 
> Can we please drop the port (which hasn't been updated since 2017)?
> 
> Best
> Martin
> 

It has been de-abandoned upstream, there is a new release from earlier this
year. Update diff for that below FWIW. It builds, runtime not tested, I
have forgotten how to use it.

(btw it has never been installed SUID from ports/packages).

The scripts in smstools3 use formail from the procmail distribution and
I never found a working alternative (though I don't run the sms-email
gateway I was using it for any more and all the mobile broadband devices
I have available are either umb or completely unsupported in OpenBSD so
I can't spin it up again to test..) That could be split off like we did
for lockfile though.


Index: Makefile
===
RCS file: /cvs/ports/mail/procmail/Makefile,v
retrieving revision 1.45
diff -u -p -r1.45 Makefile
--- Makefile11 Mar 2022 19:34:53 -  1.45
+++ Makefile4 May 2022 22:57:57 -
@@ -1,23 +1,15 @@
 COMMENT=   filtering local mail delivery agent
 
-DISTNAME=  procmail-3.22
-CATEGORIES=mail
-REVISION=  8
-
-MASTER_SITES=  ${HOMEPAGE} \
-   http://mirror.switch.ch/ftp/mirror/procmail/ \
-   http://ftp.kfki.hu/packages/mail/procmail/ \
-   http://ftp.ucsb.edu/pub/mirrors/procmail/ \
-   http://www.ring.gr.jp/archives/net/mail/procmail/ \
-   ftp://ftp.informatik.rwth-aachen.de/pub/packages/procmail/ \
-   ftp://ftp.fu-berlin.de/pub/unix/mail/procmail/
+GH_ACCOUNT=BuGlessRB
+GH_PROJECT=procmail
+GH_TAGNAME=v3.24
 
-HOMEPAGE=  http://www.procmail.org/
+CATEGORIES=mail
 
 # GPLv2+
 PERMIT_PACKAGE=Yes
 
-WANTLIB=   c m
+WANTLIB=   c m
 
 FLAVORS=   lmtp
 FLAVOR?=
Index: distinfo
===
RCS file: /cvs/ports/mail/procmail/distinfo,v
retrieving revision 1.4
diff -u -p -r1.4 distinfo
--- distinfo18 Jan 2015 03:14:25 -  1.4
+++ distinfo4 May 2022 22:57:57 -
@@ -1,2 +1,2 @@
-SHA256 (procmail-3.22.tar.gz) = CHx1s03TPYud9a/p5CgByTlfS/Nzp4TZvJcVOwBi4Rc=
-SIZE (procmail-3.22.tar.gz) = 226817
+SHA256 (procmail-3.24.tar.gz) = UU6kMzOXg+ld+TIeeUdx5Ih7mCOsVf2yRpcCz2m9OYk=
+SIZE (procmail-3.24.tar.gz) = 299704
Index: patches/patch-man_procmailrc_man
===
RCS file: patches/patch-man_procmailrc_man
diff -N patches/patch-man_procmailrc_man
--- patches/patch-man_procmailrc_man11 Mar 2022 19:34:53 -  1.2
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,14 +0,0 @@
-Fix escaping error that causes information loss.
-
-Index: man/procmailrc.man
 man/procmailrc.man.orig
-+++ man/procmailrc.man
-@@ -779,7 +779,7 @@ one trailing newline will be stripped.
- .PP
- Some non-optimal and non-obvious regexps set MATCH to an incorrect
- value.  The regexp can be made to work by removing one or more unneeded
--'*', '+', or '?' operator on the left-hand side of the \e/ token.
-+\&'*', '+', or '?' operator on the left-hand side of the \e/ token.
- .SH MISCELLANEOUS
- If the regular expression contains `\fB@TO_key@\fP' it will be substituted by
- .na
Index: patches/patch-src_comsat_c
===
RCS file: patches/patch-src_comsat_c
diff -N patches/patch-src_comsat_c
--- patches/patch-src_comsat_c  11 Mar 2022 19:34:53 -  1.3
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,15 +0,0 @@
-A patch from Philip Guenther (procmail maintainer) fixing a
-crash when procmail is invoked without arguments and then
-receive a ^C.
-
 src/comsat.c.orig  Tue Sep 11 06:55:46 2001
-+++ src/comsat.c   Wed Dec  2 23:13:21 2009
-@@ -120,7 +120,7 @@ void sendcomsat(folder)const char*folder;
- { int s;const char*p;
-   if(!csvalid||!buf)/* is comat on and set to a valid address? */
-  return;
--  if(!*cslgname||strlen(cslgname)+2>linebuf) /* is $LOGNAME bogus? */
-+  if(!cslgname||!*cslgname||strlen(cslgname)+2>linebuf)/* is $LOGNAME bogus? 
*/
-  return;
-   if(!(p=folder?folder:cslastf))   /* do we have a folder? */
- 

Re: mail/procmail CVE-2017-16844

2022-05-04 Thread Martin Schröder
Am Mi., 6. Dez. 2017 um 13:06 Uhr schrieb Stuart Henderson
:
> OK for the fix. But guenther@'s comment from 2015 still stands -
>
> "Executive summary: delete the procmail port; the code is not safe and
> should not be used as a basis for any further work."
>
> (https://marc.info/?l=openbsd-ports=141634350915839=2)

See also https://anarc.at/blog/2022-03-02-procmail-considered-harmful/

"TL;DR: procmail is a security liability and has been abandoned upstream
for the last two decades. If you are still using it, you should probably drop
everything and at least remove its SUID flag. There are plenty of alternatives
to choose from, and conversion is a one-time, acceptable trade-off."

Can we please drop the port (which hasn't been updated since 2017)?

Best
Martin



Re: mail/procmail CVE-2017-16844

2017-12-06 Thread Stuart Henderson
On 2017/12/06 12:46, Alexander Bluhm wrote:
> On Wed, Nov 29, 2017 at 09:02:07PM +0100, Stefan Sperling wrote:
> > > > + void loadbuf(text,len)const char*const text;const size_t len;
> > > > +-{ if(buffilled+len>buflen)  /* buf can't hold the 
> > > > text */
> > > > ++{ while(buffilled+len>buflen)   /* buf can't hold the 
> > > > text */
> > > > +  buf=realloc(buf,buflen+=Bsize);
> > > > +   tmemmove(buf+buffilled,text,len);buffilled+=len;
> > > > + }
> > > 
> > > Is this the real coding style ?!?!? seriously ?
> > > 
> > 
> > Yes. Quoting guenther@:
> > 
> > "Any whitespace you see is either left-indent, right-indent,
> > or syntactically required."
> 
> Any other comments beside the upstream coding style?
> 
> Otherwise I would just commit it.
> 
> bluhm
> 

OK for the fix. But guenther@'s comment from 2015 still stands -

"Executive summary: delete the procmail port; the code is not safe and 
should not be used as a basis for any further work."

(https://marc.info/?l=openbsd-ports=141634350915839=2)

AFAIK the other work necessary in the ports tree to do this has now
been done.



Re: mail/procmail CVE-2017-16844

2017-12-06 Thread Alexander Bluhm
On Wed, Nov 29, 2017 at 09:02:07PM +0100, Stefan Sperling wrote:
> > > + void loadbuf(text,len)const char*const text;const size_t len;
> > > +-{ if(buffilled+len>buflen)/* buf can't hold the 
> > > text */
> > > ++{ while(buffilled+len>buflen) /* buf can't hold the 
> > > text */
> > > +  buf=realloc(buf,buflen+=Bsize);
> > > +   tmemmove(buf+buffilled,text,len);buffilled+=len;
> > > + }
> > 
> > Is this the real coding style ?!?!? seriously ?
> > 
> 
> Yes. Quoting guenther@:
> 
> "Any whitespace you see is either left-indent, right-indent,
> or syntactically required."

Any other comments beside the upstream coding style?

Otherwise I would just commit it.

bluhm



Re: mail/procmail CVE-2017-16844

2017-11-29 Thread Stefan Sperling
On Wed, Nov 29, 2017 at 06:08:00PM +0100, Landry Breuil wrote:
> On Wed, Nov 29, 2017 at 06:03:31PM +0100, Alexander Bluhm wrote:
> > Hi,
> > 
> > +Index: src/formisc.c
> > +--- src/formisc.c.orig
> >  src/formisc.c
> >  @@ -84,12 +84,11 @@ normal:   *target++= *start++;
> > case '"':*target++=delim='"';start++;
> > }
> > @@ -19,6 +24,15 @@ with unbalanced quotes.
> > }
> >hitspc=2;
> >  }
> > +@@ -104,7 +103,7 @@ void loadsaved(sp)const struct saved*const sp;  /*
> > + }
> > +   /* append to buf */
> > + void loadbuf(text,len)const char*const text;const size_t len;
> > +-{ if(buffilled+len>buflen)  /* buf can't hold the 
> > text */
> > ++{ while(buffilled+len>buflen)   /* buf can't hold the 
> > text */
> > +  buf=realloc(buf,buflen+=Bsize);
> > +   tmemmove(buf+buffilled,text,len);buffilled+=len;
> > + }
> 
> Is this the real coding style ?!?!? seriously ?
> 

Yes. Quoting guenther@:

"Any whitespace you see is either left-indent, right-indent,
or syntactically required."



Re: mail/procmail CVE-2017-16844

2017-11-29 Thread Landry Breuil
On Wed, Nov 29, 2017 at 06:03:31PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> +Index: src/formisc.c
> +--- src/formisc.c.orig
>  src/formisc.c
>  @@ -84,12 +84,11 @@ normal: *target++= *start++;
>   case '"':*target++=delim='"';start++;
> }
> @@ -19,6 +24,15 @@ with unbalanced quotes.
> }
>hitspc=2;
>  }
> +@@ -104,7 +103,7 @@ void loadsaved(sp)const struct saved*const sp;/*
> + }
> + /* append to buf */
> + void loadbuf(text,len)const char*const text;const size_t len;
> +-{ if(buffilled+len>buflen)/* buf can't hold the text */
> ++{ while(buffilled+len>buflen) /* buf can't hold the 
> text */
> +  buf=realloc(buf,buflen+=Bsize);
> +   tmemmove(buf+buffilled,text,len);buffilled+=len;
> + }

Is this the real coding style ?!?!? seriously ?