Review of past security vulnerabilities wrt v4.70.0

[utkarsh2102]

Hello @dbry,

I am incredibly thankful for your help for CVE-2020-35738 wrt 4.70.0. As mentioned there already, I am fixing all the pending security vulnerabilities for Debian ELTS which is at version 4.70.0.

I have carefully reviewed and backported all that I could find. Here's my take on this:

CVE ID	Fixing Commit	Backported?
CVE-2016-10169	4bc05fc	Only a part of it, i.e., the hunk in src/read_words.c of the fixing commit. The initial hunk wasn't a part of the older version.
CVE-2018-19840	070ef6f	Backported completely.
CVE-2018-19841	bba5389	My assessment was the v4.70.0 was not affected as I couldn't find the vulnerable code. Let me know if this isn't the case.
CVE-2019-1010315	4c0faba	Vulnerable code is not present; DFF support was introduced later.
CVE-2019-1010317	f68a955	Vulnerable code is not present; CAF support was introduced later.
CVE-2019-1010319	33a0025	It looks like CLEAR (WaveHeader); was already in v4.70.0. So I guess this is not affecting that version. Right?
CVE-2019-11498	bc6cba3	Vulnerable code is not present; DFF support was introduced later.
CVE-2020-35738	63f3ec7 and 89df160	Backported already with your help! ❤️

If you have some time (and can take a quick look at this), could you help me verify that this is indeed correct?
Based on this, I'll also backport to v5.0.0, where some of these are already fixed.

[utkarsh2102]

Here's the entire diff that I've backported (fixing CVE-2016-10169, CVE-2018-19840, CVE-2019-1010319, and CVE-2020-35738).

--- a/src/words.c
+++ b/src/words.c
@@ -1146,6 +1146,10 @@
 
     low &= 0x7fffffff;
     high &= 0x7fffffff;
+
+    if (low > high)         // make sure high and low make sense
+        high = low;
+
     mid = (high + low + 1) >> 1;
 
     if (!c->error_limit)
--- a/src/wputils.c
+++ b/src/wputils.c
@@ -947,6 +947,21 @@
     int num_chans = config->num_channels;
     int i;
 
+    if (config->sample_rate <= 0) {
+        strcpy (wpc->error_message, "sample rate cannot be zero or negative!");
+        return FALSE;
+    }
+
+    if (num_chans <= 0 || num_chans > NEW_MAX_STREAMS * 2) {
+        strcpy (wpc->error_message, "invalid channel count!");
+        return FALSE;
+    }
+
+    if (config->block_samples && (config->block_samples < 16 || config->block_samples > 131072)) {
+        strcpy (wpc->error_message, "invalid custom block samples!");
+        return FALSE;
+    }
+
     wpc->total_samples = total_samples;
     wpc->config.sample_rate = config->sample_rate;
     wpc->config.num_channels = config->num_channels;
@@ -1096,10 +1111,10 @@
     else
         wpc->block_samples = wpc->config.sample_rate;
 
-    while (wpc->block_samples * wpc->config.num_channels > 150000)
+    while ((int64_t) wpc->block_samples * wpc->config.num_channels > 150000)
         wpc->block_samples /= 2;
 
-    while (wpc->block_samples * wpc->config.num_channels < 40000)
+    while ((int64_t) wpc->block_samples * wpc->config.num_channels < 40000)
         wpc->block_samples *= 2;
 
     if (wpc->config.block_samples) {

[dbry]

Oops, I guess I should have looked at this issue first. But some of my comments on the other issue still apply. This patch is fine.

As to CVE-2018-19841, yes, that code is checking the block checksums that were added with version 5.0.0, so no equivalent code existed before that.

[utkarsh2102]

Oops, I guess I should have looked at this issue first.

Woah, this is my bad. There was a glitch while I was opening this issue and it now seems like it got opened twice by me. Apologies for this and the noise! 😞

As to CVE-2018-19841, yes, that code is checking the block checksums that were added with version 5.0.0, so no equivalent code existed before that.

Perfect, thank you so much for all your help again!
I've fixed all the vulnerabilities in both Debian Jessie (v4.70.0) and Debian Stretch (v5.0.0).

The vulnerabilities in the later versions which are in the Debian archive are already fixed!