Replace all uses of strcpy*, strcat* with memcpy

Memcpy is always faster than any of these function when the size
to copy is known, which is always the case in this code.

In addition, most of the previous calls were vulnerable to race
conditions, where the attacker would set the final zero byte of
the string to a non-zero value after the call to strlen and
before the call to strcpy, strcpy_s, strcat or strcat_s. It's not
clear whether this was exploitable or not, but I think we should
fix this no matter what.

Google internal cl: 531243392

Change-Id: Idb7b345f27ef0c85958269f4ad75bc06316ba066

2 files changed, 35 insertions, 60 deletions

diff --git a/mkvmuxer/mkvmuxer.cc b/mkvmuxer/mkvmuxer.cc
index 0552e12..21e51be 100644
--- a/mkvmuxer/mkvmuxer.cc
+++ b/mkvmuxer/mkvmuxer.cc
@@ -65,7 +65,8 @@ bool StrCpy(const char* src, char** dst_ptr) {
   if (dst == NULL)
     return false;
 
-  strcpy(dst, src);  // NOLINT
+  memcpy(dst, src, size - 1);
+  dst[size - 1] = '\0';
   return true;
 }
 
@@ -919,11 +920,8 @@ void Track::set_codec_id(const char* codec_id) {
     const size_t length = strlen(codec_id) + 1;
     codec_id_ = new (std::nothrow) char[length];  // NOLINT
     if (codec_id_) {
-#ifdef _MSC_VER
-      strcpy_s(codec_id_, length, codec_id);
-#else
-      strcpy(codec_id_, codec_id);
-#endif
+      memcpy(codec_id_, codec_id, length - 1);
+      codec_id_[length - 1] = '\0';
     }
   }
 }
@@ -936,11 +934,8 @@ void Track::set_language(const char* language) {
     const size_t length = strlen(language) + 1;
     language_ = new (std::nothrow) char[length];  // NOLINT
     if (language_) {
-#ifdef _MSC_VER
-      strcpy_s(language_, length, language);
-#else
-      strcpy(language_, language);
-#endif
+      memcpy(language_, language, length - 1);
+      language_[length - 1] = '\0';
     }
   }
 }
@@ -952,11 +947,8 @@ void Track::set_name(const char* name) {
     const size_t length = strlen(name) + 1;
     name_ = new (std::nothrow) char[length];  // NOLINT
     if (name_) {
-#ifdef _MSC_VER
-      strcpy_s(name_, length, name);
-#else
-      strcpy(name_, name);
-#endif
+      memcpy(name_, name, length - 1);
+      name_[length - 1] = '\0';
     }
   }
 }
@@ -1559,11 +1551,8 @@ void VideoTrack::set_colour_space(const char* colour_space) {
     const size_t length = strlen(colour_space) + 1;
     colour_space_ = new (std::nothrow) char[length];  // NOLINT
     if (colour_space_) {
-#ifdef _MSC_VER
-      strcpy_s(colour_space_, length, colour_space);
-#else
-      strcpy(colour_space_, colour_space);
-#endif
+      memcpy(colour_space_, colour_space, length - 1);
+      colour_space_[length - 1] = '\0';
     }
   }
 }
@@ -2927,11 +2916,8 @@ bool SegmentInfo::Init() {
   if (!muxing_app_)
     return false;
 
-#ifdef _MSC_VER
-  strcpy_s(muxing_app_, app_len, temp);
-#else
-  strcpy(muxing_app_, temp);
-#endif
+  memcpy(muxing_app_, temp, app_len - 1);
+  muxing_app_[app_len - 1] = '\0';
 
   set_writing_app(temp);
   if (!writing_app_)
@@ -3022,11 +3008,8 @@ void SegmentInfo::set_muxing_app(const char* app) {
     if (!temp_str)
       return;
 
-#ifdef _MSC_VER
-    strcpy_s(temp_str, length, app);
-#else
-    strcpy(temp_str, app);
-#endif
+    memcpy(temp_str, app, length - 1);
+    temp_str[length - 1] = '\0';
 
     delete[] muxing_app_;
     muxing_app_ = temp_str;
@@ -3040,11 +3023,8 @@ void SegmentInfo::set_writing_app(const char* app) {
     if (!temp_str)
       return;
 
-#ifdef _MSC_VER
-    strcpy_s(temp_str, length, app);
-#else
-    strcpy(temp_str, app);
-#endif
+    memcpy(temp_str, app, length - 1);
+    temp_str[length - 1] = '\0';
 
     delete[] writing_app_;
     writing_app_ = temp_str;
@@ -3628,19 +3608,17 @@ bool Segment::SetChunking(bool chunking, const char* filename) {
     if (chunking_ && !strcmp(filename, chunking_base_name_))
       return true;
 
-    const size_t name_length = strlen(filename) + 1;
-    char* const temp = new (std::nothrow) char[name_length];  // NOLINT
+    const size_t filename_length = strlen(filename);
+    char* const temp = new (std::nothrow) char[filename_length + 1];  // NOLINT
     if (!temp)
       return false;
 
-#ifdef _MSC_VER
-    strcpy_s(temp, name_length, filename);
-#else
-    strcpy(temp, filename);
-#endif
+    memcpy(temp, filename, filename_length);
+    temp[filename_length] = '\0';
 
     delete[] chunking_base_name_;
     chunking_base_name_ = temp;
+    // From this point, strlen(chunking_base_name_) == filename_length
 
     if (!UpdateChunkName("chk", &chunk_name_))
       return false;
@@ -3666,18 +3644,16 @@ bool Segment::SetChunking(bool chunking, const char* filename) {
     if (!chunk_writer_cluster_->Open(chunk_name_))
       return false;
 
-    const size_t header_length = strlen(filename) + strlen(".hdr") + 1;
+    const size_t hdr_length = strlen(".hdr");
+    const size_t header_length = filename_length + hdr_length + 1;
     char* const header = new (std::nothrow) char[header_length];  // NOLINT
     if (!header)
       return false;
 
-#ifdef _MSC_VER
-    strcpy_s(header, header_length - strlen(".hdr"), chunking_base_name_);
-    strcat_s(header, header_length, ".hdr");
-#else
-    strcpy(header, chunking_base_name_);
-    strcat(header, ".hdr");
-#endif
+    memcpy(header, chunking_base_name_, filename_length);
+    memcpy(&header[filename_length], ".hdr", hdr_length);
+    header[filename_length + hdr_length] = '\0';
+
     if (!chunk_writer_header_->Open(header)) {
       delete[] header;
       return false;
@@ -4022,18 +3998,16 @@ bool Segment::UpdateChunkName(const char* ext, char** name) const {
   snprintf(ext_chk, sizeof(ext_chk), "_%06d.%s", chunk_count_, ext);
 #endif
 
-  const size_t length = strlen(chunking_base_name_) + strlen(ext_chk) + 1;
+  const size_t chunking_base_name_length = strlen(chunking_base_name_);
+  const size_t ext_chk_length = strlen(ext_chk);
+  const size_t length = chunking_base_name_length + ext_chk_length + 1;
   char* const str = new (std::nothrow) char[length];  // NOLINT
   if (!str)
     return false;
 
-#ifdef _MSC_VER
-  strcpy_s(str, length - strlen(ext_chk), chunking_base_name_);
-  strcat_s(str, length, ext_chk);
-#else
-  strcpy(str, chunking_base_name_);
-  strcat(str, ext_chk);
-#endif
+  memcpy(str, chunking_base_name_, chunking_base_name_length);
+  memcpy(&str[chunking_base_name_length], ext_chk, ext_chk_length);
+  str[chunking_base_name_length + ext_chk_length] = '\0';
 
   delete[] * name;
   *name = str;
diff --git a/mkvparser/mkvparser.cc b/mkvparser/mkvparser.cc
index 868afcb..35b4762 100644
--- a/mkvparser/mkvparser.cc
+++ b/mkvparser/mkvparser.cc
@@ -4569,7 +4569,8 @@ int Track::Info::CopyStr(char* Info::*str, Info& dst_) const {
   if (dst == NULL)
     return -1;
 
-  strcpy(dst, src);
+  memcpy(dst, src, len);
+  dst[len] = '\0';
 
   return 0;
 }