shithub: sox

Info  •  Files  •  Log  •  Branches

Fix (m)compand modifying their arguments

The compand and mcompand effects modified their arguments while
parsing them (via strtok(), which replaces characters by NUL). This
caused two different problems:

1. For libSoX clients with hardcoded effect arguments given as C
string literals, the program could crash during a call to
sox_effect_options() since those literals may be in write-protected
memory. Ryo Okamoto discovered this bug and provided code to
reproduce it.

2. In SoX, a compand/mcompand effect in a restarted effects chain
could see different arguments the second time it was created. E.g.,
  sox -n -n trim 0 10 compand 0.2,0.5 -6 stats : restart
failed at restart because the compand effect now saw "0.2" instead of
"0.2,0.5" as its first argument, which is a syntax error.

The fix duplicates the affected arguments and uses the copy for
further processing.

--- a/ChangeLog

+++ b/ChangeLog

@@ -40,6 +40,7 @@

   o Fix memory leaks in LADSPA effect. (Eric Wong)

   o Fix hang in several effects (rate, tempo, and those based on

     dft_filter) when processing long files. [3592482, 3594822] (MrMod)

+  o Prevent (m)compand from tampering with their arguments. (Ulrich Klauer)

 Other bug fixes:

--- a/src/compand.c

+++ b/src/compand.c

@@ -58,6 +58,10 @@

   ptrdiff_t delay_buf_index; /* Index into delay_buf */

   ptrdiff_t delay_buf_cnt; /* No. of active entries in delay_buf */

   int delay_buf_full;       /* Shows buffer situation (important for drain) */

+

+  char *arg0;  /* copies of arguments, so that they may be modified */

+  char *arg1;

+  char *arg2;

 } priv_t;

 static int getopts(sox_effect_t * effp, int argc, char * * argv)

@@ -71,8 +75,12 @@

   if (argc < 2 || argc > 5)

     return lsx_usage(effp);

+  l->arg0 = lsx_strdup(argv[0]);

+  l->arg1 = lsx_strdup(argv[1]);

+  l->arg2 = argc > 2 ? lsx_strdup(argv[2]) : NULL;

+

   /* Start by checking the attack and decay rates */

-  for (s = argv[0], commas = 0; *s; ++s) if (*s == ',') ++commas;

+  for (s = l->arg0, commas = 0; *s; ++s) if (*s == ',') ++commas;

   if ((commas % 2) == 0) {

     lsx_fail("there must be an even number of attack/decay parameters");

     return SOX_EOF;

@@ -83,7 +91,7 @@

   /* Now tokenise the rates string and set up these arrays.  Keep

      them in seconds at the moment: we don't know the sample rate yet. */

-  for (i = 0, s = strtok(argv[0], ","); s != NULL; ++i) {

+  for (i = 0, s = strtok(l->arg0, ","); s != NULL; ++i) {

     for (j = 0; j < 2; ++j) {

       if (sscanf(s, "%lf %c", &l->channels[i].attack_times[j], &dummy) != 1) {

         lsx_fail("syntax error trying to read attack/decay time");

@@ -96,7 +104,7 @@

     }

   }

-  if (!lsx_compandt_parse(&l->transfer_fn, argv[1], argc>2 ? argv[2] : 0))

+  if (!lsx_compandt_parse(&l->transfer_fn, l->arg1, l->arg2))

     return SOX_EOF;

   /* Set the initial "volume" to be attibuted to the input channels.

@@ -269,6 +277,9 @@

   lsx_compandt_kill(&l->transfer_fn);

   free(l->channels);

+  free(l->arg0);

+  free(l->arg1);

+  free(l->arg2);

   return SOX_SUCCESS;

 }

--- a/src/mcompand.c

+++ b/src/mcompand.c

@@ -72,6 +72,8 @@

   size_t band_buf_len;

   size_t delay_buf_size;/* Size of delay_buf in samples */

   comp_band_t *bands;

+

+  char *arg; /* copy of current argument */

 } priv_t;

 /*

@@ -174,7 +176,7 @@

   /* how many bands? */

   if (! (argc&1)) {

     lsx_fail("mcompand accepts only an odd number of arguments:\argc"

-            "  mcompand quoted_compand_args [crossover_freq quoted_compand_args [...]");

+            "  mcompand quoted_compand_args [crossover_freq quoted_compand_args [...]]");

     return SOX_EOF;

   }

   c->nBands = (argc+1)>>1;

@@ -182,10 +184,13 @@

   c->bands = lsx_calloc(c->nBands, sizeof(comp_band_t));

   for (i=0;i<c->nBands;++i) {

-    if (parse_subarg(argv[i<<1],subargv,&subargc) != SOX_SUCCESS)

+    c->arg = lsx_strdup(argv[i<<1]);

+    if (parse_subarg(c->arg,subargv,&subargc) != SOX_SUCCESS)

       return SOX_EOF;

     if (sox_mcompand_getopts_1(&c->bands[i], subargc, &subargv[0]) != SOX_SUCCESS)

       return SOX_EOF;

+    free(c->arg);

+    c->arg = NULL;

     if (i == (c->nBands-1))

       c->bands[i].topfreq = 0;

     else {

@@ -493,6 +498,7 @@

     free(l->attackRate);

     free(l->volume);

   }

+  free(c->arg);

   free(c->bands);

   c->bands = NULL;