Handle Issues that could be triggered by malicious/corrupt data

CMakeLists.txt

@@ -21,13 +21,13 @@ add_library(${PROJECT_NAME} ${source_files})
 
 if (TINYWAV_ALLOCATION MATCHES "ALLOCA")
   message(STATUS "Configuring tinywav to use ALLOCA for allocations")
-  target_compile_definitions(${PROJECT_NAME} PRIVATE TINYWAV_USE_ALLOCA=1)
+  target_compile_definitions(${PROJECT_NAME} PUBLIC TINYWAV_USE_ALLOCA=1)
 elseif(TINYWAV_ALLOCATION MATCHES "VLA")
   message(STATUS "Configuring tinywav to use VLA for allocations")
-  target_compile_definitions(${PROJECT_NAME} PRIVATE TINYWAV_USE_VLA=1)
+  target_compile_definitions(${PROJECT_NAME} PUBLIC TINYWAV_USE_VLA=1)
 elseif(TINYWAV_ALLOCATION MATCHES "MALLOC")
   message(STATUS "Configuring tinywav to use MALLOC for allocations")
-  target_compile_definitions(${PROJECT_NAME} PRIVATE TINYWAV_USE_MALLOC=1)
+  target_compile_definitions(${PROJECT_NAME} PUBLIC TINYWAV_USE_MALLOC=1)
 else()
   message(FATAL_ERROR "Invalid option for TINYWAV_ALLOCATION -- valid options are: ALLOCA VLA MALLOC")
 endif()

README.md

@@ -9,8 +9,9 @@ A minimal C library for reading and writing (32-bit float or 16-bit int) WAV aud
 
 * TinyWav takes and provides audio samples in configurable channel formats (interleaved, split, inline). WAV files always store samples in interleaved format.
 * TinyWav is minimal: it can only read/write RIFF WAV files with sample format `float32` or `int16`.
-* TinyWav does not allocate any memory on the heap. It uses `alloca` internally, which allocates on the stack. In practice, this restricts the block size to "reasonable" values, so watch out for stack overflows.
-   * On platforms where `alloca` is not available (e.g. some DSP compilers), `TINYWAV_USE_VLA` or `TINYWAV_USE_MALLOC` can be defined.
+* TinyWav does not allocate any memory on the heap. It uses `alloca` internally, which allocates on the stack. In practice, this restricts the block size to "reasonable" values, so watch out for stack overflows. 
+	* In default `TINYWAV_USE_ALLOCA` mode, the `read()` and `write()` will return an error if the allocation exceeds 0.5 MegaBytes, which corresponds e.g. to reading/writing 8k 32-bit sample chunks for 16 channels.
+   * On platforms where `alloca` is not available (e.g. some DSP compilers), `TINYWAV_USE_VLA` or `TINYWAV_USE_MALLOC` can be defined for alternate allocation.
 
 **CI/CD**: To guarantee portability, TinyWav is built and tested on several platforms, compilers & architectures:
 

test/tests/BasicTests.cpp

@@ -224,7 +224,7 @@ TEST_CASE("Tinywav - Test Error Behaviour")
     float buffer[128];
     REQUIRE(tinywav_read_f(&tw, buffer, -1) != 0);
     REQUIRE(tinywav_read_f(&tw, buffer, 0) == 0);
-    REQUIRE(tinywav_read_f(&tw, buffer, 16) == 0); // no data in file yet!
+    REQUIRE(tinywav_read_f(&tw, buffer, 16) == -1); // no data in file yet!
     tinywav_close_read(&tw);
 
     // Test data

test/tests/MaliciousFileTests.cpp

@@ -0,0 +1,72 @@
+
+#include <catch2/catch.hpp>
+#include "tinywav.h"
+
+#include <cstring> // for memset
+#include "TestCommon.hpp"
+
+
+static auto basedir = std::string(TOSTRING(SOURCE_DIR)) + "/test/test-data/";
+
+TEST_CASE("Tinywav - Test behaviour with malicious input data")
+{
+  TinyWav tw;
+  // without proper handling, this test could trigger a division by zero
+  REQUIRE(tinywav_open_read(&tw, std::string(basedir + "malicious_numChannels0.wav").c_str(), TW_INTERLEAVED) == -1);
+  tinywav_close_read(&tw);
+}
+
+TEST_CASE("Tinywav - Test Safeguards")
+{
+  SECTION("Reading") {
+    TinyWav tw;
+    REQUIRE(tinywav_open_read(&tw, std::string(basedir + "example_32bitFloat-mono.wav").c_str(), TW_INTERLEAVED) == 0);
+    int numFramesToRead = tw.numFramesInHeader;
+
+    SECTION("try to read all samples declared in header at once") {
+      float* buffer = (float*)malloc(numFramesToRead*tw.numChannels*sizeof(float));
+      REQUIRE(tinywav_read_f(&tw, buffer, numFramesToRead) == numFramesToRead);
+      free(buffer);
+    }
+    SECTION("try to read more samples than declared in header") {
+      numFramesToRead += 4; // too much!
+      float* buffer = nullptr; // should fail before trying to write to buffer
+      REQUIRE(tinywav_read_f(&tw, buffer, numFramesToRead) == -1);
+    }
+#if TINYWAV_USE_ALLOCA
+    SECTION("trigger alloca safeguard") {
+      float* buffer = nullptr; // should fail before trying to write to buffer
+      tw.numChannels = 32; // overwrite with another number of channels to trigger safeguard
+      REQUIRE(tinywav_read_f(&tw, buffer, numFramesToRead) == -1);
+    }
+#endif
+
+    tinywav_close_read(&tw);
+  }
+
+
+  SECTION("writing") {
+    if (TestCommon::fileExists("bogus.wav")) {
+      REQUIRE(std::remove("bogus.wav") == 0);
+    }
+
+    TinyWav tw;
+    REQUIRE(tinywav_open_write(&tw, 16, 8000, TW_FLOAT32, TW_INLINE, "bogus.wav") == 0);
+    int maxAllowedNumFrames16ch = 8*1024; // max 16ch, 8kSamples
+
+    SECTION("try to write max samples") {
+      float* buffer = (float*)malloc(maxAllowedNumFrames16ch*tw.numChannels*sizeof(float));
+      REQUIRE(tinywav_write_f(&tw, buffer, maxAllowedNumFrames16ch) == maxAllowedNumFrames16ch);
+      free(buffer);
+    }
+#if TINYWAV_USE_ALLOCA
+    SECTION("trigger alloca safeguard") {
+      maxAllowedNumFrames16ch += 4; // too much!
+      float* buffer = nullptr; // should fail before trying to write to buffer
+      REQUIRE(tinywav_write_f(&tw, buffer, maxAllowedNumFrames16ch) == -1);
+    }
+#endif
+
+    tinywav_close_write(&tw);
+  }
+}

tinywav.c

@@ -50,6 +50,9 @@
     #define TW_DEALLOC(x)
 #endif
 
+// Corresponds to 0.5MB for 32bit samples --> allows max of 16ch, 8kSamples blocksize reads/writes
+static const size_t REASONABLE_MAX_ALLOCA_SIZE = 16*8*1024; // in samples
+
 // MARK: private functions
 
 /** @returns true if the chunk of 4 characters matches the supplied string */
@@ -195,6 +198,16 @@ int tinywav_open_read(TinyWav *tw, const char *path, TinyWavChannelFormat chanFm
     return -1;
   }
 
+  // Sanity checks
+  if (tw->h.NumChannels < 1 || tw->h.NumChannels > 128) { // relevant because
+      tinywav_close_read(tw);
+      return -1;
+  }
+  if (tw->h.SampleRate < 1) {
+      tinywav_close_read(tw);
+      return -1;
+  }
+
   // skip over any other chunks before the "data" chunk (e.g. JUNK, INFO, bext, ...)
   while (fread(tw->h.Subchunk2ID, sizeof(char), 4, tw->f) == 4) {
     fread(&tw->h.Subchunk2Size, sizeof(uint32_t), 1, tw->f);
@@ -217,6 +230,7 @@ int tinywav_open_read(TinyWav *tw, const char *path, TinyWavChannelFormat chanFm
     printf("[tinywav] Warning: wav file has %d bits per sample (int), which is not natively supported yet. Treating them as float; you may want to convert them manually after reading.\n", tw->h.BitsPerSample);
   }
 
+  // NOTE: previous sanity checks ensure div by zero is not possible here
   tw->numFramesInHeader = tw->h.Subchunk2Size / (tw->numChannels * tw->sampFmt);
   tw->totalFramesReadWritten = 0;
 
@@ -228,6 +242,14 @@ int tinywav_read_f(TinyWav *tw, void *data, int len) {
   if (tw == NULL || data == NULL || len < 0 || !tinywav_isOpen(tw)) {
     return -1;
   }
+  if (len > tw->numFramesInHeader) {
+    return -1;
+  }
+#if TINYWAV_USE_ALLOCA
+  if (((size_t)tw->numChannels * len) > REASONABLE_MAX_ALLOCA_SIZE) {
+      return -1;
+  }
+#endif
 
   if (tw->totalFramesReadWritten * tw->h.BlockAlign >= tw->h.Subchunk2Size) {
     // We are past the 'data' subchunk (size as declared in header).
@@ -330,6 +352,11 @@ int tinywav_write_f(TinyWav *tw, void *f, int len) {
   if (tw == NULL || f == NULL || len < 0 || !tinywav_isOpen(tw)) {
     return -1;
   }
+#if TINYWAV_USE_ALLOCA
+  if (((size_t)tw->numChannels * len) > REASONABLE_MAX_ALLOCA_SIZE) {
+      return -1;
+  }
+#endif
 
   // 1. Bring samples into interleaved format
   // 2. write to disk