Addressed Tavis's comments in http://codereview.chromium.org/3023041/....

src/cff_type2_charstring.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // A parser for the Type 2 Charstring Format.
 // http://www.adobe.com/devnet/font/pdfs/5177.Type2.pdf
 
 #include "cff_type2_charstring.h"
 
+#include <climits>
 #include <cstdio>
 #include <cstring>
-#include <limits>
 #include <stack>
 #include <string>
 #include <utility>
 
 namespace {
 
-// The list of Operators. See Appendix. A in Adobe Technical Note #5177.
-enum Type2CharStringOperator {
-  kHStem = 1,
-  kVStem = 3,
-  kVMoveTo = 4,
-  kRLineTo = 5,
-  kHLineTo = 6,
-  kVLineTo = 7,
-  kRRCurveTo = 8,
-  kCallSubr = 10,
-  kReturn = 11,
-  kEndChar = 14,
-  kHStemHm = 18,
-  kHintMask = 19,
-  kCntrMask = 20,
-  kRMoveTo = 21,
-  kHMoveTo = 22,
-  kVStemHm = 23,
-  kRCurveLine = 24,
-  kRLineCurve = 25,
-  kVVCurveTo = 26,
-  kHHCurveTo = 27,
-  kCallGSubr = 29,
-  kVHCurveTo = 30,
-  kHVCurveTo = 31,
-  kAnd = (12 << 8) + 3,
-  kOr = (12 << 8) + 4,
-  kNot = (12 << 8) + 5,
-  kAbs = (12 << 8) + 9,
-  kAdd = (12 << 8) + 10,
-  kSub = (12 << 8) + 11,
-  kDiv = (12 << 8) + 12,
-  kNeg = (12 << 8) + 14,
-  kEq = (12 << 8) + 15,
-  kDrop = (12 << 8) + 18,
-  kPut = (12 << 8) + 20,
-  kGet = (12 << 8) + 21,
-  kIfElse = (12 << 8) + 22,
-  kRandom = (12 << 8) + 23,
-  kMul = (12 << 8) + 24,
-  kSqrt = (12 << 8) + 26,
-  kDup = (12 << 8) + 27,
-  kExch = (12 << 8) + 28,
-  kIndex = (12 << 8) + 29,
-  kRoll = (12 << 8) + 30,
-  kHFlex = (12 << 8) + 34,
-  kFlex = (12 << 8) + 35,
-  kHFlex1 = (12 << 8) + 36,
-  kFlex1 = (12 << 8) + 37,
-};
-
 // Type 2 Charstring Implementation Limits. See Appendix. B in Adobe Technical
 // Note #5177.
+const int32_t kMaxSubrsCount = 65536;
 const size_t kMaxCharStringLength = 65535;
 const size_t kMaxArgumentStack = 48;
 const size_t kMaxNumberOfStemHints = 96;
 const size_t kMaxSubrNesting = 10;
 
+// |dummy_result| should be a huge positive integer so callsubr and callgsubr
+// will fail with the dummy value.
+const int32_t dummy_result = INT_MAX;
+
 bool ExecuteType2CharString(size_t call_depth,
                             const ots::CFFIndex& global_subrs_index,
                             const ots::CFFIndex& local_subrs_index,
                             ots::Buffer *cff_table,
                             ots::Buffer *char_string,
                             std::stack<int32_t> *argument_stack,
                             bool *out_found_endchar,
                             bool *out_found_width,
                             size_t *in_out_num_stems);
 
 // Converts |op| to a string and returns it.
-const char *Type2CharStringOperatorToString(Type2CharStringOperator op) {
+const char *Type2CharStringOperatorToString(ots::Type2CharStringOperator op) {
   switch (op) {
-  case kHStem:
+  case ots::kHStem:
     return "HStem";
-  case kVStem:
+  case ots::kVStem:
     return "VStem";
-  case kVMoveTo:
+  case ots::kVMoveTo:
     return "VMoveTo";
-  case kRLineTo:
+  case ots::kRLineTo:
     return "RLineTo";
-  case kHLineTo:
+  case ots::kHLineTo:
     return "HLineTo";
-  case kVLineTo:
+  case ots::kVLineTo:
     return "VLineTo";
-  case kRRCurveTo:
+  case ots::kRRCurveTo:
     return "RRCurveTo";
-  case kCallSubr:
+  case ots::kCallSubr:
     return "CallSubr";
-  case kReturn:
+  case ots::kReturn:
     return "Return";
-  case kEndChar:
+  case ots::kEndChar:
     return "EndChar";
-  case kHStemHm:
+  case ots::kHStemHm:
     return "HStemHm";
-  case kHintMask:
+  case ots::kHintMask:
     return "HintMask";
-  case kCntrMask:
+  case ots::kCntrMask:
     return "CntrMask";
-  case kRMoveTo:
+  case ots::kRMoveTo:
     return "RMoveTo";
-  case kHMoveTo:
+  case ots::kHMoveTo:
     return "HMoveTo";
-  case kVStemHm:
+  case ots::kVStemHm:
     return "VStemHm";
-  case kRCurveLine:
+  case ots::kRCurveLine:
     return "RCurveLine";
-  case kRLineCurve:
+  case ots::kRLineCurve:
     return "RLineCurve";
-  case kVVCurveTo:
+  case ots::kVVCurveTo:
     return "VVCurveTo";
-  case kHHCurveTo:
+  case ots::kHHCurveTo:
     return "HHCurveTo";
-  case kCallGSubr:
+  case ots::kCallGSubr:
     return "CallGSubr";
-  case kVHCurveTo:
+  case ots::kVHCurveTo:
     return "VHCurveTo";
-  case kHVCurveTo:
+  case ots::kHVCurveTo:
     return "HVCurveTo";
-  case kAnd:
+  case ots::kAnd:
     return "And";
-  case kOr:
+  case ots::kOr:
     return "Or";
-  case kNot:
+  case ots::kNot:
     return "Not";
-  case kAbs:
+  case ots::kAbs:
     return "Abs";
-  case kAdd:
+  case ots::kAdd:
     return "Add";
-  case kSub:
+  case ots::kSub:
     return "Sub";
-  case kDiv:
+  case ots::kDiv:
     return "Div";
-  case kNeg:
+  case ots::kNeg:
     return "Neg";
-  case kEq:
+  case ots::kEq:
     return "Eq";
-  case kDrop:
+  case ots::kDrop:
     return "Drop";
-  case kPut:
+  case ots::kPut:
     return "Put";
-  case kGet:
+  case ots::kGet:
     return "Get";
-  case kIfElse:
+  case ots::kIfElse:
     return "IfElse";
-  case kRandom:
+  case ots::kRandom:
     return "Random";
-  case kMul:
+  case ots::kMul:
     return "Mul";
-  case kSqrt:
+  case ots::kSqrt:
     return "Sqrt";
-  case kDup:
+  case ots::kDup:
     return "Dup";
-  case kExch:
+  case ots::kExch:
     return "Exch";
-  case kIndex:
+  case ots::kIndex:
     return "Index";
-  case kRoll:
+  case ots::kRoll:
     return "Roll";
-  case kHFlex:
+  case ots::kHFlex:
     return "HFlex";
-  case kFlex:
+  case ots::kFlex:
     return "Flex";
-  case kHFlex1:
+  case ots::kHFlex1:
     return "HFlex1";
-  case kFlex1:
+  case ots::kFlex1:
     return "Flex1";
   }
 
   return "UNKNOWN";
 }
 
 // Read one or more bytes from the |char_string| buffer and stores the number
 // read on |out_number|. If the number read is an operator (ex 'vstem'), sets
 // true on |out_is_operator|. Returns true if the function read a number.
 bool ReadNextNumberFromType2CharString(ots::Buffer *char_string,
@@ skipping 46 matching lines @@
     *out_number = ((static_cast<int32_t>(v) - 247) * 256) +
         static_cast<int32_t>(w) + 108;
   } else if (v <= 254) {
     uint8_t w = 0;
     if (!char_string->ReadU8(&w)) {
       return OTS_FAILURE();
     }
     *out_number = -((static_cast<int32_t>(v) - 251) * 256) -
         static_cast<int32_t>(w) - 108;
   } else if (v == 255) {
-    uint32_t result = 0;
-    for (size_t i = 0; i < 4; ++i) {
-      if (!char_string->ReadU8(&v)) {
-        return OTS_FAILURE();
-      }
-      result = (result << 8) + v;
+    // TODO(yusukes): We should not skip the 4 bytes. Note that when v is 255,
+    // we should treat the following 4-bytes as a 16.16 fixed-point number
+    // rather than 32bit signed int.
+    if (!char_string->Skip(4)) {
+      return OTS_FAILURE();
     }
-    // TODO(yusukes): |result| should be treated as 16.16 fixed-point number
-    // rather than 32bit signed int.
-    *out_number = static_cast<int32_t>(result);
+    *out_number = dummy_result;
   } else {
     return OTS_FAILURE();
   }
 
   return true;
 }
 
 // Executes |op| and updates |argument_stack|. Returns true if the execution
 // succeeds. If the |op| is kCallSubr or kCallGSubr, the function recursively
 // calls ExecuteType2CharString() function. The arguments other than |op| and
 // |argument_stack| are passed for that reason.
 bool ExecuteType2CharStringOperator(int32_t op,
                                     size_t call_depth,
                                     const ots::CFFIndex& global_subrs_index,
                                     const ots::CFFIndex& local_subrs_index,
                                     ots::Buffer *cff_table,
                                     ots::Buffer *char_string,
                                     std::stack<int32_t> *argument_stack,
                                     bool *out_found_endchar,
                                     bool *in_out_found_width,
                                     size_t *in_out_num_stems) {
-  // |dummy_result| should be a huge positive integer so callsubr and callgsubr
-  // will fail with the dummy value.
-  const int32_t dummy_result = std::numeric_limits<int>::max();
   const size_t stack_size = argument_stack->size();
 
   switch (op) {
-  case kCallSubr:
-  case kCallGSubr: {
+  case ots::kCallSubr:
+  case ots::kCallGSubr: {
     const ots::CFFIndex& subrs_index =
-      (op == kCallSubr ? local_subrs_index : global_subrs_index);
+        (op == ots::kCallSubr ? local_subrs_index : global_subrs_index);
 
     if (stack_size < 1) {
       return OTS_FAILURE();
     }
     int32_t subr_number = argument_stack->top();
     argument_stack->pop();
     if (subr_number == dummy_result) {
       // For safety, we allow subr calls only with immediate subr numbers for
       // now. For example, we allow "123 callgsubr", but does not allow "100 12
       // add callgsubr". Please note that arithmetic and conditional operators
       // always push the |dummy_result| in this implementation.
       return OTS_FAILURE();
     }
 
     // See Adobe Technical Note #5176 (CFF), "16. Local/GlobalSubrs INDEXes."
     int32_t bias = 32768;
     if (subrs_index.count < 1240) {
       bias = 107;
     } else if (subrs_index.count < 33900) {
       bias = 1131;
     }
     subr_number += bias;
 
     // Sanity checks of |subr_number|.
     if (subr_number < 0) {
       return OTS_FAILURE();
     }
-    if (subrs_index.offsets.size() <= static_cast<size_t>(subr_number)) {
+    if (subr_number >= kMaxSubrsCount) {
+      return OTS_FAILURE();
+    }
+    if (subrs_index.offsets.size() <= static_cast<size_t>(subr_number + 1)) {
       return OTS_FAILURE();  // The number is out-of-bounds.
     }
 
     // Prepare ots::Buffer where we're going to jump.
     const size_t length =
       subrs_index.offsets[subr_number + 1] - subrs_index.offsets[subr_number];
     if (length > kMaxCharStringLength) {
       return OTS_FAILURE();
     }
     const size_t offset = subrs_index.offsets[subr_number];
     cff_table->set_offset(offset);
     if (!cff_table->Skip(length)) {
       return OTS_FAILURE();
     }
     ots::Buffer char_string_to_jump(cff_table->buffer() + offset, length);
 
     return ExecuteType2CharString(call_depth + 1,
                                   global_subrs_index,
                                   local_subrs_index,
                                   cff_table,
                                   &char_string_to_jump,
                                   argument_stack,
                                   out_found_endchar,
                                   in_out_found_width,
                                   in_out_num_stems);
   }
 
-  case kReturn:
+  case ots::kReturn:
     return true;
 
-  case kEndChar:
+  case ots::kEndChar:
     *out_found_endchar = true;
     *in_out_found_width = true;  // just in case.
     return true;
 
-  case kHStem:
-  case kVStem:
-  case kHStemHm:
-  case kVStemHm: {
+  case ots::kHStem:
+  case ots::kVStem:
+  case ots::kHStemHm:
+  case ots::kVStemHm: {
     bool successful = false;
     if (stack_size < 2) {
       return OTS_FAILURE();
     }
     if ((stack_size % 2) == 0) {
       successful = true;
     } else if ((!(*in_out_found_width)) && (((stack_size - 1) % 2) == 0)) {
       // The -1 is for "width" argument. For details, see Adobe Technical Note
       // #5177, page 16, note 4.
       successful = true;
     }
     (*in_out_num_stems) += (stack_size / 2);
     if ((*in_out_num_stems) > kMaxNumberOfStemHints) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     *in_out_found_width = true;  // always set true since "w" might be 0 byte.
     return successful ? true : OTS_FAILURE();
   }
 
-  case kRMoveTo: {
+  case ots::kRMoveTo: {
     bool successful = false;
     if (stack_size == 2) {
       successful = true;
     } else if ((!(*in_out_found_width)) && (stack_size - 1 == 2)) {
       successful = true;
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     *in_out_found_width = true;
     return successful ? true : OTS_FAILURE();
   }
 
-  case kVMoveTo:
-  case kHMoveTo: {
+  case ots::kVMoveTo:
+  case ots::kHMoveTo: {
     bool successful = false;
     if (stack_size == 1) {
       successful = true;
     } else if ((!(*in_out_found_width)) && (stack_size - 1 == 1)) {
       successful = true;
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     *in_out_found_width = true;
     return successful ? true : OTS_FAILURE();
   }
 
-  case kHintMask:
-  case kCntrMask: {
+  case ots::kHintMask:
+  case ots::kCntrMask: {
     bool successful = false;
     if (stack_size == 0) {
       successful = true;
     } else if ((!(*in_out_found_width)) && (stack_size == 1)) {
       // A number for "width" is found.
       successful = true;
     } else if ((!(*in_out_found_width)) ||  // in this case, any sizes are ok.
                ((stack_size % 2) == 0)) {
       // The numbers are vstem definition.
       // See Adobe Technical Note #5177, page 24, hintmask.
@@ skipping 13 matching lines @@
     const size_t mask_bytes = (*in_out_num_stems + 7) / 8;
     if (!char_string->Skip(mask_bytes)) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     *in_out_found_width = true;
     return true;
   }
 
-  case kRLineTo:
+  case ots::kRLineTo:
     if (!(*in_out_found_width)) {
       // The first stack-clearing operator should be one of hstem, hstemhm,
       // vstem, vstemhm, cntrmask, hintmask, hmoveto, vmoveto, rmoveto, or
       // endchar. For details, see Adobe Technical Note #5177, page 16, note 4.
       return OTS_FAILURE();
     }
     if (stack_size < 2) {
       return OTS_FAILURE();
     }
     if ((stack_size % 2) != 0) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
 
-  case kHLineTo:
-  case kVLineTo:
+  case ots::kHLineTo:
+  case ots::kVLineTo:
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size < 1) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
 
-  case kRRCurveTo:
+  case ots::kRRCurveTo:
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size < 6) {
       return OTS_FAILURE();
     }
     if ((stack_size % 6) != 0) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
 
-  case kRCurveLine:
+  case ots::kRCurveLine:
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size < 8) {
       return OTS_FAILURE();
     }
     if (((stack_size - 2) % 6) != 0) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
 
-  case kRLineCurve:
+  case ots::kRLineCurve:
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size < 8) {
       return OTS_FAILURE();
     }
     if (((stack_size - 6) % 2) != 0) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
 
-  case kVVCurveTo:
+  case ots::kVVCurveTo:
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size < 4) {
       return OTS_FAILURE();
     }
     if (((stack_size % 4) != 0) &&
         (((stack_size - 1) % 4) != 0)) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
 
-  case kHHCurveTo: {
+  case ots::kHHCurveTo: {
     bool successful = false;
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size < 4) {
       return OTS_FAILURE();
     }
     if ((stack_size % 4) == 0) {
       // {dxa dxb dyb dxc}+
       successful = true;
     } else if (((stack_size - 1) % 4) == 0) {
       // dy1? {dxa dxb dyb dxc}+
       successful = true;
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return successful ? true : OTS_FAILURE();
   }
 
-  case kVHCurveTo:
-  case kHVCurveTo: {
+  case ots::kVHCurveTo:
+  case ots::kHVCurveTo: {
     bool successful = false;
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size < 4) {
       return OTS_FAILURE();
     }
     if (((stack_size - 4) % 8) == 0) {
       // dx1 dx2 dy2 dy3 {dya dxb dyb dxc dxd dxe dye dyf}*
       successful = true;
     } else if ((stack_size >= 5) &&
                ((stack_size - 5) % 8) == 0) {
       // dx1 dx2 dy2 dy3 {dya dxb dyb dxc dxd dxe dye dyf}* dxf
       successful = true;
     } else if ((stack_size >= 8) &&
                ((stack_size - 8) % 8) == 0) {
       // {dxa dxb dyb dyc dyd dxe dye dxf}+
       successful = true;
     } else if ((stack_size >= 9) &&
                ((stack_size - 9) % 8) == 0) {
       // {dxa dxb dyb dyc dyd dxe dye dxf}+ dyf?
       successful = true;
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return successful ? true : OTS_FAILURE();
   }
 
-  case kAnd:
-  case kOr:
-  case kEq:
-  case kAdd:
-  case kSub:
+  case ots::kAnd:
+  case ots::kOr:
+  case ots::kEq:
+  case ots::kAdd:
+  case ots::kSub:
     if (stack_size < 2) {
       return OTS_FAILURE();
     }
     argument_stack->pop();
     argument_stack->pop();
     argument_stack->push(dummy_result);
     // TODO(yusukes): Implement this. We should push a real value for all
     // arithmetic and conditional operations.
     return true;
 
-  case kNot:
-  case kAbs:
-  case kNeg:
+  case ots::kNot:
+  case ots::kAbs:
+  case ots::kNeg:
     if (stack_size < 1) {
       return OTS_FAILURE();
     }
     argument_stack->pop();
     argument_stack->push(dummy_result);
     // TODO(yusukes): Implement this. We should push a real value for all
     // arithmetic and conditional operations.
     return true;
 
-  case kDiv:
+  case ots::kDiv:
     // TODO(yusukes): Should detect div-by-zero errors.
-    if (stack_size < 1) {
+    if (stack_size < 2) {
       return OTS_FAILURE();
     }
     argument_stack->pop();
+    argument_stack->pop();
     argument_stack->push(dummy_result);
     // TODO(yusukes): Implement this. We should push a real value for all
     // arithmetic and conditional operations.
     return true;
 
-  case kDrop:
+  case ots::kDrop:
     if (stack_size < 1) {
       return OTS_FAILURE();
     }
     argument_stack->pop();
     return true;
 
-  case kPut:
-  case kGet:
-  case kIndex:
+  case ots::kPut:
+  case ots::kGet:
+  case ots::kIndex:
     // For now, just call OTS_FAILURE since there is no way to check whether the
     // index argument, |i|, is out-of-bounds or not. Fortunately, no OpenType
     // fonts I have (except malicious ones!) use the operators.
     // TODO(yusukes): Implement them in a secure way.
     return OTS_FAILURE();
 
-  case kRoll:
+  case ots::kRoll:
     // Likewise, just call OTS_FAILURE for kRoll since there is no way to check
     // whether |N| is smaller than the current stack depth or not.
     // TODO(yusukes): Implement them in a secure way.
     return OTS_FAILURE();
 
-  case kRandom:
+  case ots::kRandom:
     // For now, we don't handle the 'random' operator since the operator makes
     // it hard to analyze hinting code statically.
     return OTS_FAILURE();
 
-  case kIfElse:
+  case ots::kIfElse:
     if (stack_size < 4) {
       return OTS_FAILURE();
     }
     argument_stack->pop();
     argument_stack->pop();
     argument_stack->pop();
     argument_stack->pop();
     argument_stack->push(dummy_result);
     // TODO(yusukes): Implement this. We should push a real value for all
     // arithmetic and conditional operations.
     return true;
 
-  case kMul:
+  case ots::kMul:
     // TODO(yusukes): Should detect overflows.
     if (stack_size < 2) {
       return OTS_FAILURE();
     }
     argument_stack->pop();
     argument_stack->pop();
     argument_stack->push(dummy_result);
     // TODO(yusukes): Implement this. We should push a real value for all
     // arithmetic and conditional operations.
     return true;
 
-  case kSqrt:
+  case ots::kSqrt:
     // TODO(yusukes): Should check if the argument is negative.
     if (stack_size < 1) {
       return OTS_FAILURE();
     }
     argument_stack->pop();
     argument_stack->push(dummy_result);
     // TODO(yusukes): Implement this. We should push a real value for all
     // arithmetic and conditional operations.
     return true;
 
-  case kDup:
+  case ots::kDup:
     if (stack_size < 1) {
       return OTS_FAILURE();
     }
     argument_stack->pop();
     argument_stack->push(dummy_result);
     argument_stack->push(dummy_result);
     if (argument_stack->size() > kMaxArgumentStack) {
       return OTS_FAILURE();
     }
     // TODO(yusukes): Implement this. We should push a real value for all
     // arithmetic and conditional operations.
     return true;
 
-  case kExch:
+  case ots::kExch:
     if (stack_size < 2) {
       return OTS_FAILURE();
     }
     argument_stack->pop();
     argument_stack->pop();
     argument_stack->push(dummy_result);
     argument_stack->push(dummy_result);
     // TODO(yusukes): Implement this. We should push a real value for all
     // arithmetic and conditional operations.
     return true;
 
-  case kHFlex:
+  case ots::kHFlex:
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size != 7) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
 
-  case kFlex:
+  case ots::kFlex:
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size != 13) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
 
-  case kHFlex1:
+  case ots::kHFlex1:
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size != 9) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
 
-  case kFlex1:
+  case ots::kFlex1:
     if (!(*in_out_found_width)) {
       return OTS_FAILURE();
     }
     if (stack_size != 11) {
       return OTS_FAILURE();
     }
     while (!argument_stack->empty())
       argument_stack->pop();
     return true;
   }
@@ skipping 70 matching lines @@
                                         char_string,
                                         argument_stack,
                                         out_found_endchar,
                                         in_out_found_width,
                                         in_out_num_stems)) {
       return OTS_FAILURE();
     }
     if (*out_found_endchar) {
       return true;
     }
-    if (operator_or_operand == kReturn) {
+    if (operator_or_operand == ots::kReturn) {
       return true;
     }
   }
 
   // No endchar operator is found.
   return OTS_FAILURE();
 }
 
 // Selects a set of subroutings for |glyph_index| from |cff| and sets it on
 // |out_local_subrs_to_use|. Returns true on success.
@@ skipping 82 matching lines @@
     std::stack<int32_t> argument_stack;
     bool found_endchar = false;
     bool found_width = false;
     size_t num_stems = 0;
     if (!ExecuteType2CharString(0 /* initial call_depth is zero */,
                                 global_subrs_index, *local_subrs_to_use,
                                 cff_table, &char_string, &argument_stack,
                                 &found_endchar, &found_width, &num_stems)) {
       return OTS_FAILURE();
     }
+    if (!found_endchar) {
+      return OTS_FAILURE();
+    }
   }
   return true;
 }
 
 }  // namespace ots