diff-match-patch-cpp-stl icon indicating copy to clipboard operation
diff-match-patch-cpp-stl copied to clipboard

The critical bug causing crash is revealed in method diff_match_patch::diff_charsToLines

Open ddomoskanov opened this issue 2 years ago • 0 comments

Running the algorythm under fuzzing tool has revealed the conversion issue from signed integer to bigger size unsigned integer. It causes heap-buffer-overflow and further crash:

static void diff_charsToLines(Diffs &diffs, const Lines& lineArray) {
    for (typename Diffs::iterator cur_diff = diffs.begin(); cur_diff != diffs.end(); ++cur_diff) {
      string_t text;
      for (int y = 0; y < (int)(*cur_diff).text.length(); y++) {
        const LinePtr& lp = lineArray[static_cast<size_t>((*cur_diff).text[y])]; <= HERE IS THE PROBLEM
        text.append(lp.first, lp.second);
      }
      (*cur_diff).text.swap(text);
    }
  }

The following fix needs to be applied in order to address. The point is that we should first convert to unsigned of the same size and then its safe to cast to size_t:

  static void diff_charsToLines(Diffs &diffs, const Lines& lineArray) {
    for (typename Diffs::iterator cur_diff = diffs.begin(); cur_diff != diffs.end(); ++cur_diff) {
      string_t text;
      for (int y = 0; y < (int)(*cur_diff).text.length(); y++) {
          typedef typename std::make_unsigned<typename string_t::value_type>::type unsigned_value_type;              <= FIX
          const LinePtr& lp = lineArray[static_cast<size_t>(static_cast<unsigned_value_type>((*cur_diff).text[y]))]; <= FIX
          text.append(lp.first, lp.second);
      }
      (*cur_diff).text.swap(text);
    }
  }

ddomoskanov avatar May 05 '22 14:05 ddomoskanov