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

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;

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;
 }