Re: audio/sox patches from oss-security

2023-02-07 Thread Theo Buehler
On Tue, Feb 07, 2023 at 02:48:25PM +0100, Jan Stary wrote:
> On Feb 06 22:56:02, t...@theobuehler.org wrote:
> > There is an ongoing discussion on audio/sox on oss-security:
> > 
> > https://marc.info/?l=oss-security=167546008232629=2
> > 
> > Steffen Nurpmeso ported the patches to apply against the commit
> > we also use in our ports, that's what's included in the diff below.
> > 
> > The patches look sensible to me although I haven't reviewed them
> > thoroughly.
> > 
> > It's probably a good idea to keep an eye on this discussion both for
> > reviews of the patches and for possible developments of a new upstream
> > repo containing them.
> 
> I just asked upstream - let's wait a on whether the upstream maintainer
> decides to include these in the upstream git (SF) that we build from;
> I would prefer that to maintaining the patches (thank you Steffen!)

For reference:

https://marc.info/?l=sox-devel=167577672104072=2

The patches were sent to oss-security *because* upstream failed to react:

   I am working on fixing known vulnerabilities in sox and since upstream
   seems mostly dead (no commits in more than a year, no replies to bug
   reports)

Given this, waiting until upstream decides to unhibernate makes little
sense to me.



Re: audio/sox patches from oss-security

2023-02-07 Thread Jan Stary
On Feb 06 22:56:02, t...@theobuehler.org wrote:
> There is an ongoing discussion on audio/sox on oss-security:
> 
> https://marc.info/?l=oss-security=167546008232629=2
> 
> Steffen Nurpmeso ported the patches to apply against the commit
> we also use in our ports, that's what's included in the diff below.
> 
> The patches look sensible to me although I haven't reviewed them
> thoroughly.
> 
> It's probably a good idea to keep an eye on this discussion both for
> reviews of the patches and for possible developments of a new upstream
> repo containing them.

I just asked upstream - let's wait a on whether the upstream maintainer
decides to include these in the upstream git (SF) that we build from;
I would prefer that to maintaining the patches (thank you Steffen!)

Jan


> ===
> RCS file: /cvs/ports/audio/sox/Makefile,v
> retrieving revision 1.74
> diff -u -p -r1.74 Makefile
> --- Makefile  11 Mar 2022 18:20:31 -  1.74
> +++ Makefile  6 Feb 2023 21:39:18 -
> @@ -5,6 +5,7 @@ V=14.4.2pl20210509
>  GIT_V=   14.4.3git
>  DISTNAME=sox-${V}
>  SHARED_LIBS +=   sox 4.1 # 3.0
> +REVISION =   0
>  
>  CATEGORIES=  audio
>  HOMEPAGE=http://sox.sourceforge.net/
> Index: patches/patch-src_aiff_c
> ===
> RCS file: patches/patch-src_aiff_c
> diff -N patches/patch-src_aiff_c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-src_aiff_c  6 Feb 2023 21:38:58 -
> @@ -0,0 +1,17 @@
> +https://marc.info/?l=oss-security=167571683504082=2
> +
> +Index: src/aiff.c
> +--- src/aiff.c.orig
>  src/aiff.c
> +@@ -619,6 +619,11 @@ int lsx_aiffstartwrite(sox_format_t * ft)
> +At 48 kHz, 16 bits stereo, this gives ~3 hours of audio.
> +Sorry, the AIFF format does not provide for an indefinite
> +number of samples. */
> ++if (ft->signal.channels >= (0x7f00 / 
> (ft->encoding.bits_per_sample >> 3)))
> ++{
> ++lsx_fail_errno(ft, SOX_EOF, "too many channels for AIFF 
> header");
> ++return SOX_EOF;
> ++}
> + return(aiffwriteheader(ft, (uint64_t) 0x7f00 / 
> ((ft->encoding.bits_per_sample>>3)*ft->signal.channels)));
> + }
> + 
> Index: patches/patch-src_formats_c
> ===
> RCS file: /cvs/ports/audio/sox/patches/patch-src_formats_c,v
> retrieving revision 1.8
> diff -u -p -r1.8 patch-src_formats_c
> --- patches/patch-src_formats_c   11 Mar 2022 18:20:31 -  1.8
> +++ patches/patch-src_formats_c   6 Feb 2023 21:38:58 -
> @@ -1,3 +1,5 @@
> +https://marc.info/?l=oss-security=167571683504082=2
> +
>  Index: src/formats.c
>  --- src/formats.c.orig
>  +++ src/formats.c
> @@ -19,3 +21,11 @@ Index: src/formats.c
>   char * command = lsx_malloc(strlen(command_format) + 
> strlen(identifier));
>   sprintf(command, command_format, identifier);
>   f = popen(command, POPEN_MODE);
> +@@ -627,6 +627,7 @@ error:
> +   free(ft->priv);
> +   free(ft->filename);
> +   free(ft->filetype);
> ++  sox_delete_comments(>oob.comments);
> +   free(ft);
> +   return NULL;
> + }
> Index: patches/patch-src_formats_i_c
> ===
> RCS file: patches/patch-src_formats_i_c
> diff -N patches/patch-src_formats_i_c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-src_formats_i_c 6 Feb 2023 21:38:58 -
> @@ -0,0 +1,42 @@
> +https://marc.info/?l=oss-security=167571683504082=2
> +
> +Index: src/formats_i.c
> +--- src/formats_i.c.orig
>  src/formats_i.c
> +@@ -19,6 +19,7 @@
> +  */
> + 
> + #include "sox_i.h"
> ++#include 
> + #include 
> + #include 
> + #include 
> +@@ -60,13 +61,24 @@ int lsx_check_read_params(sox_format_t * ft, unsigned 
> +   if (ft->seekable)
> + ft->data_start = lsx_tell(ft);
> + 
> +-  if (channels && ft->signal.channels && ft->signal.channels != channels)
> ++  if (channels && ft->signal.channels && ft->signal.channels != channels) {
> + lsx_warn("`%s': overriding number of channels", ft->filename);
> +-  else ft->signal.channels = channels;
> ++  } else if (channels > SHRT_MAX) {
> ++lsx_fail_errno(ft, EINVAL, "implausibly large number of channels");
> ++return SOX_EOF;
> ++  } else {
> ++ft->signal.channels = channels;
> ++  }
> + 
> +-  if (rate && ft->signal.rate && ft->signal.rate != rate)
> ++  if (rate && ft->signal.rate && ft->signal.rate != rate) {
> + lsx_warn("`%s': overriding sample rate", ft->filename);
> +-  else ft->signal.rate = rate;
> ++  /* Since NaN comparisons yield false, the negation rejects them. */
> ++  } else if (!(rate > 0)) {
> ++lsx_fail_errno(ft, EINVAL, "invalid rate value");
> ++return SOX_EOF;
> ++  } else {
> ++ft->signal.rate = rate;
> ++  }
> + 
> +   if (encoding && ft->encoding.encoding && 

audio/sox patches from oss-security

2023-02-06 Thread Theo Buehler
There is an ongoing discussion on audio/sox on oss-security:

https://marc.info/?l=oss-security=167546008232629=2

Steffen Nurpmeso ported the patches to apply against the commit
we also use in our ports, that's what's included in the diff below.

The patches look sensible to me although I haven't reviewed them
thoroughly.

It's probably a good idea to keep an eye on this discussion both for
reviews of the patches and for possible developments of a new upstream
repo containing them.

Index: Makefile
===
RCS file: /cvs/ports/audio/sox/Makefile,v
retrieving revision 1.74
diff -u -p -r1.74 Makefile
--- Makefile11 Mar 2022 18:20:31 -  1.74
+++ Makefile6 Feb 2023 21:39:18 -
@@ -5,6 +5,7 @@ V=  14.4.2pl20210509
 GIT_V= 14.4.3git
 DISTNAME=  sox-${V}
 SHARED_LIBS += sox 4.1 # 3.0
+REVISION = 0
 
 CATEGORIES=audio
 HOMEPAGE=  http://sox.sourceforge.net/
Index: patches/patch-src_aiff_c
===
RCS file: patches/patch-src_aiff_c
diff -N patches/patch-src_aiff_c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-src_aiff_c6 Feb 2023 21:38:58 -
@@ -0,0 +1,17 @@
+https://marc.info/?l=oss-security=167571683504082=2
+
+Index: src/aiff.c
+--- src/aiff.c.orig
 src/aiff.c
+@@ -619,6 +619,11 @@ int lsx_aiffstartwrite(sox_format_t * ft)
+At 48 kHz, 16 bits stereo, this gives ~3 hours of audio.
+Sorry, the AIFF format does not provide for an indefinite
+number of samples. */
++if (ft->signal.channels >= (0x7f00 / 
(ft->encoding.bits_per_sample >> 3)))
++{
++lsx_fail_errno(ft, SOX_EOF, "too many channels for AIFF 
header");
++return SOX_EOF;
++}
+ return(aiffwriteheader(ft, (uint64_t) 0x7f00 / 
((ft->encoding.bits_per_sample>>3)*ft->signal.channels)));
+ }
+ 
Index: patches/patch-src_formats_c
===
RCS file: /cvs/ports/audio/sox/patches/patch-src_formats_c,v
retrieving revision 1.8
diff -u -p -r1.8 patch-src_formats_c
--- patches/patch-src_formats_c 11 Mar 2022 18:20:31 -  1.8
+++ patches/patch-src_formats_c 6 Feb 2023 21:38:58 -
@@ -1,3 +1,5 @@
+https://marc.info/?l=oss-security=167571683504082=2
+
 Index: src/formats.c
 --- src/formats.c.orig
 +++ src/formats.c
@@ -19,3 +21,11 @@ Index: src/formats.c
  char * command = lsx_malloc(strlen(command_format) + strlen(identifier));
  sprintf(command, command_format, identifier);
  f = popen(command, POPEN_MODE);
+@@ -627,6 +627,7 @@ error:
+   free(ft->priv);
+   free(ft->filename);
+   free(ft->filetype);
++  sox_delete_comments(>oob.comments);
+   free(ft);
+   return NULL;
+ }
Index: patches/patch-src_formats_i_c
===
RCS file: patches/patch-src_formats_i_c
diff -N patches/patch-src_formats_i_c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-src_formats_i_c   6 Feb 2023 21:38:58 -
@@ -0,0 +1,42 @@
+https://marc.info/?l=oss-security=167571683504082=2
+
+Index: src/formats_i.c
+--- src/formats_i.c.orig
 src/formats_i.c
+@@ -19,6 +19,7 @@
+  */
+ 
+ #include "sox_i.h"
++#include 
+ #include 
+ #include 
+ #include 
+@@ -60,13 +61,24 @@ int lsx_check_read_params(sox_format_t * ft, unsigned 
+   if (ft->seekable)
+ ft->data_start = lsx_tell(ft);
+ 
+-  if (channels && ft->signal.channels && ft->signal.channels != channels)
++  if (channels && ft->signal.channels && ft->signal.channels != channels) {
+ lsx_warn("`%s': overriding number of channels", ft->filename);
+-  else ft->signal.channels = channels;
++  } else if (channels > SHRT_MAX) {
++lsx_fail_errno(ft, EINVAL, "implausibly large number of channels");
++return SOX_EOF;
++  } else {
++ft->signal.channels = channels;
++  }
+ 
+-  if (rate && ft->signal.rate && ft->signal.rate != rate)
++  if (rate && ft->signal.rate && ft->signal.rate != rate) {
+ lsx_warn("`%s': overriding sample rate", ft->filename);
+-  else ft->signal.rate = rate;
++  /* Since NaN comparisons yield false, the negation rejects them. */
++  } else if (!(rate > 0)) {
++lsx_fail_errno(ft, EINVAL, "invalid rate value");
++return SOX_EOF;
++  } else {
++ft->signal.rate = rate;
++  }
+ 
+   if (encoding && ft->encoding.encoding && ft->encoding.encoding != encoding)
+ lsx_warn("`%s': overriding encoding type", ft->filename);
Index: patches/patch-src_hcom_c
===
RCS file: patches/patch-src_hcom_c
diff -N patches/patch-src_hcom_c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-src_hcom_c6 Feb 2023 21:38:58 -
@@ -0,0 +1,57 @@
+https://marc.info/?l=oss-security=167571683504082=2
+
+Index: src/hcom.c
+--- src/hcom.c.orig
 src/hcom.c
+@@ -141,6 +141,11 @@ static int