CVE-2017-6838: Integer Overflow or Wraparound (CWE-190)
value: Integer overflow in sfcommands/sfconvert.c in Audio File Library (aka audiofile) 0.3.6 allows remote attackers to cause a denial of service (crash) via a crafted file.
CWE description: The product performs a calculation that can
         produce an integer overflow or wraparound when the logic
         assumes that the resulting value will always be larger than
         the original value. This occurs when an integer value is
         incremented to a value that is too large to store in the
         associated representation. When this occurs, the value may
         become a very small or negative number.
CVSS v3 base score: 5.5
Published: 2017-03-20T16:59Z
Repository: https://github.com/antlarr/audiofile
Language: C
Files changed: sfcommands/sfconvert.c
Commit date: 2017-03-06 13:57:57 +0100
Commit message: Check for multiplication overflow in sfconvert

Checks that a multiplication doesn't overflow when
calculating the buffer size, and if it overflows,
reduce the buffer size instead of failing.

This fixes the 00192-audiofile-signintoverflow-sfconvert case
in #41

Diff: diff --git a/sfcommands/sfconvert.c b/sfcommands/sfconvert.c
index 80a1bc4..970a3e4 100644
--- a/sfcommands/sfconvert.c
+++ b/sfcommands/sfconvert.c
@@ -43,10 +43,37 @@
 void printversion (void);
 void printusage (void);
 void usageerror (void);
 bool copyaudiodata (AFfilehandle infile, AFfilehandle outfile, int trackid);
 
+int firstBitSet(int x)
+{
+        int position=0;
+        while (x!=0)
+        {
+                x>>=1;
+                ++position;
+        }
+        return position;
+}
+
+#ifndef __has_builtin
+#define __has_builtin(x) 0
+#endif
+
+int multiplyCheckOverflow(int a, int b, int *result)
+{
+#if (defined __GNUC__ && __GNUC__ >= 5) || ( __clang__ && __has_builtin(__builtin_mul_overflow))
+	return __builtin_mul_overflow(a, b, result);
+#else
+	if (firstBitSet(a)+firstBitSet(b)>31) // int is signed, so we can't use 32 bits
+		return true;
+	*result = a * b;
+	return false;
+#endif
+}
+
 int main (int argc, char **argv)
 {
 	if (argc == 2)
 	{
 		if (!strcmp(argv[1], "--version") || !strcmp(argv[1], "-v"))
@@ -321,12 +348,15 @@ void printversion (void)
 */
 bool copyaudiodata (AFfilehandle infile, AFfilehandle outfile, int trackid)
 {
 	int frameSize = afGetVirtualFrameSize(infile, trackid, 1);
 
-	const int kBufferFrameCount = 65536;
-	void *buffer = malloc(kBufferFrameCount * frameSize);
+	int kBufferFrameCount = 65536;
+	int bufferSize;
+	while (multiplyCheckOverflow(kBufferFrameCount, frameSize, &bufferSize))
+		kBufferFrameCount /= 2;
+	void *buffer = malloc(bufferSize);
 
 	AFframecount totalFrames = afGetFrameCount(infile, AF_DEFAULT_TRACK);
 	AFframecount totalFramesWritten = 0;
 
 	bool success = true;

Vulnerable code: 	const int kBufferFrameCount = 65536;
	void *buffer = malloc(kBufferFrameCount * frameSize);
Fixed code: int firstBitSet(int x)
{
        int position=0;
        while (x!=0)
        {
                x>>=1;
                ++position;
        }
        return position;
}

#ifndef __has_builtin
#define __has_builtin(x) 0
#endif

int multiplyCheckOverflow(int a, int b, int *result)
{
#if (defined __GNUC__ && __GNUC__ >= 5) || ( __clang__ && __has_builtin(__builtin_mul_overflow))
	return __builtin_mul_overflow(a, b, result);
#else
	if (firstBitSet(a)+firstBitSet(b)>31) // int is signed, so we can't use 32 bits
		return true;
	*result = a * b;
	return false;
#endif
}

	int kBufferFrameCount = 65536;
	int bufferSize;
	while (multiplyCheckOverflow(kBufferFrameCount, frameSize, &bufferSize))
		kBufferFrameCount /= 2;
	void *buffer = malloc(bufferSize);