Add error checking

src/CMakeLists.txt

@@ -1,11 +1,13 @@
+add_compile_options(
+    -Werror=all
+    -Werror=extra
+
+    -Werror=shadow
+)
+
 add_executable(tzdb2nx
     main.cpp
     tzif.cpp
     tzif.h)
 
-add_compile_options(
-    -Werror=all
-    -Werror=extra
-)
-
 include_directories(.)

src/main.cpp

@@ -1,7 +1,10 @@
 #include "tzif.h"
 #include <array>
+#include <cerrno>
 #include <cstdio>
+#include <cstring>
 #include <fcntl.h>
+#include <poll.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -10,39 +13,94 @@ constexpr std::size_t ten_megabytes{(1 << 20) * 10};
 
 int main(int argc, char *argv[]) {
   int f{STDIN_FILENO};
-  char *filename{};
+  const char *filename{"(stdin)"};
   std::size_t filesize{ten_megabytes};
 
   if (argc > 1) {
-    char *filename = argv[1];
+    filename = argv[1];
     f = open(filename, O_RDONLY);
 
+    if (f == -1) {
+      const int err = errno;
+      std::fprintf(stderr, "%s: %s\n", filename, std::strerror(err));
+      return -1;
+    }
+
     struct stat statbuf;
-    stat(filename, &statbuf);
+    fstat(f, &statbuf);
 
     filesize = statbuf.st_size;
+  } else {
+    struct pollfd fds {
+      f, POLLIN, 0,
+    };
+
+    const int result = poll(&fds, 1, 0);
+    if (result == 0) {
+      std::fprintf(stderr, "%s: No input\n", filename);
+      return -1;
+    }
   }
 
   u_int8_t *buf = new u_int8_t[filesize];
 
   filesize = read(f, buf, filesize);
-  close(f);
+  if (filesize == static_cast<std::size_t>(-1)) {
+    const int err = errno;
+    std::fprintf(stderr, "%s: %s\n", filename, std::strerror(err));
+    return -1;
+  }
+  int result = close(f);
+  if (result == -1) {
+    const int err = errno;
+    std::fprintf(stderr, "%s: %s\n", filename, std::strerror(err));
+    return -1;
+  }
 
-  const std::unique_ptr<Tzif::DataImpl> tzif_data =
-      Tzif::ReadData(buf, filesize);
+  if (filesize < 4) {
+    std::fprintf(stderr, "%s: Too small\n", filename);
+    return -1;
+  }
+  if (std::strncmp(reinterpret_cast<const char *>(buf), "TZif", 4) != 0) {
+    std::fprintf(stderr, "%s: Bad magic number\n", filename);
+    return -1;
+  }
+
+  const std::unique_ptr<Tzif::Data> tzif_data = Tzif::ReadData(buf, filesize);
+  if (tzif_data == nullptr) {
+    std::fprintf(stderr, "%s: Error occured while reading data\n", filename);
+    return -1;
+  }
 
   std::vector<u_int8_t> output_buffer;
   tzif_data->ReformatNintendo(output_buffer);
 
+  filename = "(stdout)";
   f = STDOUT_FILENO;
   if (argc > 2) {
-    char *filename = argv[2];
+    filename = argv[2];
     f = open(filename, O_WRONLY | O_CREAT | O_TRUNC);
+
+    if (f == -1) {
+      const int err = errno;
+      std::fprintf(stderr, "%s: %s\n", filename, std::strerror(err));
+      return -1;
+    }
   }
 
-  write(f, output_buffer.data(), output_buffer.size());
+  result = write(f, output_buffer.data(), output_buffer.size());
+  if (result == -1) {
+    const int err = errno;
+    std::fprintf(stderr, "%s: %s\n", filename, std::strerror(err));
+    return -1;
+  }
 
-  close(f);
+  result = close(f);
+  if (result == -1) {
+    const int err = errno;
+    std::fprintf(stderr, "%s: %s\n", filename, std::strerror(err));
+    return -1;
+  }
 
   return 0;
 }

src/tzif.cpp

@@ -6,7 +6,7 @@
 
 namespace Tzif {
 
-static std::size_t SkipToVersion2(const u_int8_t *data) {
+static std::size_t SkipToVersion2(const u_int8_t *data, std::size_t size) {
   char magic[5];
   const u_int8_t *p{data};
 
@@ -14,11 +14,14 @@ static std::size_t SkipToVersion2(const u_int8_t *data) {
   magic[4] = '\0';
 
   if (std::strcmp(magic, "TZif") != 0) {
-    return 0;
+    return -1;
   }
 
   do {
     p++;
+    if (p >= data + size) {
+      return -1;
+    }
   } while (std::strncmp(reinterpret_cast<const char *>(p), "TZif", 4) != 0);
 
   return p - data;
@@ -32,8 +35,8 @@ template <typename Type> constexpr static void SwapEndianess(Type *value) {
     Type value;
   } temp;
 
-  for (int i = 0; i < sizeof(Type); i++) {
-    int alt_index = sizeof(Type) - i - 1;
+  for (u_int32_t i = 0; i < sizeof(Type); i++) {
+    u_int32_t alt_index = sizeof(Type) - i - 1;
     temp.data[alt_index] = data[i];
   }
 
@@ -50,7 +53,12 @@ static void FlipHeader(Header &header) {
 }
 
 std::unique_ptr<DataImpl> ReadData(const u_int8_t *data, std::size_t size) {
-  const u_int8_t *p = data + SkipToVersion2(data);
+  const std::size_t v2_offset = SkipToVersion2(data, size);
+  if (v2_offset == static_cast<std::size_t>(-1)) {
+    return nullptr;
+  }
+
+  const u_int8_t *p = data + v2_offset;
 
   Header header;
   std::memcpy(&header, p, sizeof(header));
@@ -58,42 +66,43 @@ std::unique_ptr<DataImpl> ReadData(const u_int8_t *data, std::size_t size) {
 
   FlipHeader(header);
 
+  const std::size_t data_block_length =
+      header.timecnt * sizeof(int64_t) + header.timecnt * sizeof(u_int8_t) +
+      header.typecnt * sizeof(TimeTypeRecord) +
+      header.charcnt * sizeof(int8_t) + header.isstdcnt * sizeof(u_int8_t) +
+      header.isutcnt * sizeof(u_int8_t);
+
+  if (v2_offset + data_block_length + sizeof(Header) > size) {
+    return nullptr;
+  }
+
   std::unique_ptr<DataImpl> impl = std::make_unique<DataImpl>();
   impl->header = header;
 
-  impl->transition_times = std::make_unique<int64_t[]>(header.timecnt);
-  impl->transition_types = std::make_unique<u_int8_t[]>(header.timecnt);
-  impl->local_time_type_records =
-      std::make_unique<TimeTypeRecord[]>(header.typecnt);
-  impl->time_zone_designations = std::make_unique<int8_t[]>(header.charcnt);
-  impl->standard_indicators = std::make_unique<u_int8_t[]>(header.isstdcnt);
-  impl->ut_indicators = std::make_unique<u_int8_t[]>(header.isutcnt);
+  const auto copy =
+      []<typename Type>(std::unique_ptr<Type[]> &array, int length,
+                        const u_int8_t *const &ptr) -> const u_int8_t * {
+    const std::size_t region_length = length * sizeof(Type);
+    array = std::make_unique<Type[]>(length);
+    std::memcpy(array.get(), ptr, region_length);
+    return ptr + region_length;
+  };
 
-  std::memcpy(impl->transition_times.get(), p,
-              header.timecnt * sizeof(int64_t));
-  p += header.timecnt * sizeof(int64_t);
-
-  std::memcpy(impl->transition_types.get(), p,
-              header.timecnt * sizeof(u_int8_t));
-  p += header.timecnt * sizeof(u_int8_t);
-
-  std::memcpy(impl->local_time_type_records.get(), p,
-              header.typecnt * sizeof(TimeTypeRecord));
-  p += header.typecnt * sizeof(TimeTypeRecord);
-
-  std::memcpy(impl->time_zone_designations.get(), p, header.charcnt);
-  p += header.charcnt * sizeof(int8_t);
-
-  std::memcpy(impl->standard_indicators.get(), p,
-              header.isstdcnt * sizeof(u_int8_t));
-  p += header.isstdcnt * sizeof(u_int8_t);
-
-  std::memcpy(impl->ut_indicators.get(), p, header.isutcnt * sizeof(u_int8_t));
-  p += header.isutcnt * sizeof(u_int8_t);
+  p = copy(impl->transition_times, header.timecnt, p);
+  p = copy(impl->transition_types, header.timecnt, p);
+  p = copy(impl->local_time_type_records, header.typecnt, p);
+  p = copy(impl->time_zone_designations, header.charcnt, p);
+  p = copy(impl->standard_indicators, header.isstdcnt, p);
+  p = copy(impl->ut_indicators, header.isutcnt, p);
 
   const std::size_t footer_string_length = data + size - p - 2;
   p++;
 
+  if (p + footer_string_length > data + size ||
+      p + footer_string_length < data) {
+    return nullptr;
+  }
+
   impl->footer.tz_string = std::make_unique<char[]>(footer_string_length);
   std::memcpy(impl->footer.tz_string.get(), p, footer_string_length);
   impl->footer.footer_string_length = footer_string_length;

src/tzif.h

@@ -44,7 +44,7 @@ static_assert(sizeof(TimeTypeRecord) == 0x6);
 class Data {
 public:
   explicit Data() = default;
-  ~Data() = default;
+  virtual ~Data() = default;
 
   virtual void ReformatNintendo(std::vector<u_int8_t> &buffer) const = 0;
 };
@@ -52,7 +52,7 @@ public:
 class DataImpl : public Data {
 public:
   explicit DataImpl() = default;
-  ~DataImpl() = default;
+  ~DataImpl() override = default;
 
   void ReformatNintendo(std::vector<u_int8_t> &buffer) const override;