Re: spectrogram segfault

[Eric Wong <normalperson@yhbt.net>]

Jan Stary <hans@stare.cz> wrote:
> It seems that if the -x size of the spectrogram is bigger
> than the number of actual samples (1000 in the example),
> or samples/second, SoX segfaults instead of rejecting that option
> (say, limiting the number to the number of samples available).

Thanks.  I think ensuring p->block_steps is always >= 1
seems acceptable; but maybe there's a better way to fix this.

------------------------8<-------------------------
Subject: [PATCH] spectrogram: fix segfault when requested size exceeds length

Thanks to a bug report by Jan Stary:

ref: http://mid.gmane.org/20150101202336.GB17144@www.stare.cz

Tested with the following Perl script based on Jan's bug report.
-------------------- spectrogram.t ----------------
use strict;
use Test::More 0.88; # 0.88+ for done_testing
use File::Temp qw/tempdir/;

my $tmpdir = tempdir('sox-spectrogram.XXXXXXXX', CLEANUP => 1);
chdir($tmpdir) or die "failed to chdir: $!\n";
END { chdir('/') }; # allow $tmpdir removal when test completes

sub cmd_ok {
	my ($msg, @cmd) = @_;
	is(system(@cmd), 0, $msg);
}

cmd_ok('create temporary test file',
	qw(sox -c 1 -b 16 -r 1000 -n file.wav synth 1 sin 440 gain -3));

cmd_ok('create PNG with default',
	qw(sox file.wav -n spectrogram));

cmd_ok('create PNG with exact -x size',
	qw(sox file.wav -n spectrogram -o b.png));

cmd_ok('create PNG with larger -x size (should not SEGV)',
	qw(sox file.wav -n spectrogram -x 1001 -o c.png));

cmd_ok('create PNG with exact -X size',
	qw(sox file.wav -n spectrogram -X 1000 -o d.png));

cmd_ok('create PNG with larger -X size (should not SEGV)',
	qw(sox file.wav -n spectrogram -X 1001 -o e.png));

done_testing();
---
 src/spectrogram.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/spectrogram.c b/src/spectrogram.c
index afb0b0e..5a2115f 100644
--- a/src/spectrogram.c
+++ b/src/spectrogram.c
@@ -275,6 +275,8 @@ static int start(sox_effect_t * effp)
   lsx_debug("window_density=%g", actual / p->dft_size);
   p->step_size = (p->slack_overlap? sqrt(actual * p->dft_size) : actual) + .5;
   p->block_steps = effp->in_signal.rate / pixels_per_sec;
+  if (p->block_steps == 0)
+    p->block_steps = 1;
   p->step_size = p->block_steps / ceil((double)p->block_steps / p->step_size) +.5;
   p->block_steps = floor((double)p->block_steps / p->step_size +.5);
   p->block_norm = 1. / p->block_steps;
-- 
EW

------------------------------------------------------------------------------