vim-patch:9.0.0577: buffer underflow with unexpected :finally

Problem:    Buffer underflow with unexpected :finally.
Solution:   Check CSF_TRY can be found.

https://github.com/vim/vim/commit/96b9bf8f74af8abf1e30054f996708db7dc285be

Co-authored-by: Bram Moolenaar <Bram@vim.org>

1 files changed, 236 insertions, 235 deletions

diff --git a/src/nvim/ex_eval.c b/src/nvim/ex_eval.c
index f696ab3900..ed83725740 100644
--- a/src/nvim/ex_eval.c
+++ b/src/nvim/ex_eval.c
@@ -1420,108 +1420,107 @@ void ex_finally(exarg_T *eap)
   int pending = CSTP_NONE;
   cstack_T *const cstack = eap->cstack;
 
-  if (cstack->cs_trylevel <= 0 || cstack->cs_idx < 0) {
-    eap->errmsg = _("E606: :finally without :try");
-  } else {
-    if (!(cstack->cs_flags[cstack->cs_idx] & CSF_TRY)) {
-      eap->errmsg = get_end_emsg(cstack);
-      for (idx = cstack->cs_idx - 1; idx > 0; idx--) {
-        if (cstack->cs_flags[idx] & CSF_TRY) {
-          break;
-        }
-      }
-      // Make this error pending, so that the commands in the following
-      // finally clause can be executed.  This overrules also a pending
-      // ":continue", ":break", ":return", or ":finish".
-      pending = CSTP_ERROR;
-    } else {
-      idx = cstack->cs_idx;
+  for (idx = cstack->cs_idx; idx >= 0; idx--) {
+    if (cstack->cs_flags[idx] & CSF_TRY) {
+      break;
     }
+  }
+  if (cstack->cs_trylevel <= 0 || idx < 0) {
+    eap->errmsg = _("E606: :finally without :try");
+    return;
+  }
 
-    if (cstack->cs_flags[idx] & CSF_FINALLY) {
-      // Give up for a multiple ":finally" and ignore it.
-      eap->errmsg = _("E607: multiple :finally");
-      return;
-    }
-    rewind_conditionals(cstack, idx, CSF_WHILE | CSF_FOR,
-                        &cstack->cs_looplevel);
+  if (!(cstack->cs_flags[cstack->cs_idx] & CSF_TRY)) {
+    eap->errmsg = get_end_emsg(cstack);
+    // Make this error pending, so that the commands in the following
+    // finally clause can be executed.  This overrules also a pending
+    // ":continue", ":break", ":return", or ":finish".
+    pending = CSTP_ERROR;
+  }
 
-    // Don't do something when the corresponding try block never got active
-    // (because of an inactive surrounding conditional or after an error or
-    // interrupt or throw) or for a ":finally" without ":try" or a multiple
-    // ":finally".  After every other error (did_emsg or the conditional
-    // errors detected above) or after an interrupt (got_int) or an
-    // exception (did_throw), the finally clause must be executed.
-    skip = !(cstack->cs_flags[cstack->cs_idx] & CSF_TRUE);
+  if (cstack->cs_flags[idx] & CSF_FINALLY) {
+    // Give up for a multiple ":finally" and ignore it.
+    eap->errmsg = _("E607: multiple :finally");
+    return;
+  }
+  rewind_conditionals(cstack, idx, CSF_WHILE | CSF_FOR,
+                      &cstack->cs_looplevel);
+
+  // Don't do something when the corresponding try block never got active
+  // (because of an inactive surrounding conditional or after an error or
+  // interrupt or throw) or for a ":finally" without ":try" or a multiple
+  // ":finally".  After every other error (did_emsg or the conditional
+  // errors detected above) or after an interrupt (got_int) or an
+  // exception (did_throw), the finally clause must be executed.
+  skip = !(cstack->cs_flags[cstack->cs_idx] & CSF_TRUE);
+
+  if (!skip) {
+    // When debugging or a breakpoint was encountered, display the
+    // debug prompt (if not already done).  The user then knows that the
+    // finally clause is executed.
+    if (dbg_check_skipped(eap)) {
+      // Handle a ">quit" debug command as if an interrupt had
+      // occurred before the ":finally".  That is, discard the
+      // original exception and replace it by an interrupt
+      // exception.
+      (void)do_intthrow(cstack);
+    }
 
-    if (!skip) {
-      // When debugging or a breakpoint was encountered, display the
-      // debug prompt (if not already done).  The user then knows that the
-      // finally clause is executed.
-      if (dbg_check_skipped(eap)) {
-        // Handle a ">quit" debug command as if an interrupt had
-        // occurred before the ":finally".  That is, discard the
-        // original exception and replace it by an interrupt
-        // exception.
-        (void)do_intthrow(cstack);
+    // If there is a preceding catch clause and it caught the exception,
+    // finish the exception now.  This happens also after errors except
+    // when this is a multiple ":finally" or one not within a ":try".
+    // After an error or interrupt, this also discards a pending
+    // ":continue", ":break", ":finish", or ":return" from the preceding
+    // try block or catch clause.
+    cleanup_conditionals(cstack, CSF_TRY, false);
+
+    // Make did_emsg, got_int, did_throw pending.  If set, they overrule
+    // a pending ":continue", ":break", ":return", or ":finish".  Then
+    // we have particularly to discard a pending return value (as done
+    // by the call to cleanup_conditionals() above when did_emsg or
+    // got_int is set).  The pending values are restored by the
+    // ":endtry", except if there is a new error, interrupt, exception,
+    // ":continue", ":break", ":return", or ":finish" in the following
+    // finally clause.  A missing ":endwhile", ":endfor" or ":endif"
+    // detected here is treated as if did_emsg and did_throw had
+    // already been set, respectively in case that the error is not
+    // converted to an exception, did_throw had already been unset.
+    // We must not set did_emsg here since that would suppress the
+    // error message.
+    if (pending == CSTP_ERROR || did_emsg || got_int || did_throw) {
+      if (cstack->cs_pending[cstack->cs_idx] == CSTP_RETURN) {
+        report_discard_pending(CSTP_RETURN,
+                               cstack->cs_rettv[cstack->cs_idx]);
+        discard_pending_return(cstack->cs_rettv[cstack->cs_idx]);
       }
-
-      // If there is a preceding catch clause and it caught the exception,
-      // finish the exception now.  This happens also after errors except
-      // when this is a multiple ":finally" or one not within a ":try".
-      // After an error or interrupt, this also discards a pending
-      // ":continue", ":break", ":finish", or ":return" from the preceding
-      // try block or catch clause.
-      cleanup_conditionals(cstack, CSF_TRY, false);
-
-      // Make did_emsg, got_int, did_throw pending.  If set, they overrule
-      // a pending ":continue", ":break", ":return", or ":finish".  Then
-      // we have particularly to discard a pending return value (as done
-      // by the call to cleanup_conditionals() above when did_emsg or
-      // got_int is set).  The pending values are restored by the
-      // ":endtry", except if there is a new error, interrupt, exception,
-      // ":continue", ":break", ":return", or ":finish" in the following
-      // finally clause.  A missing ":endwhile", ":endfor" or ":endif"
-      // detected here is treated as if did_emsg and did_throw had
-      // already been set, respectively in case that the error is not
-      // converted to an exception, did_throw had already been unset.
-      // We must not set did_emsg here since that would suppress the
-      // error message.
-      if (pending == CSTP_ERROR || did_emsg || got_int || did_throw) {
-        if (cstack->cs_pending[cstack->cs_idx] == CSTP_RETURN) {
-          report_discard_pending(CSTP_RETURN,
-                                 cstack->cs_rettv[cstack->cs_idx]);
-          discard_pending_return(cstack->cs_rettv[cstack->cs_idx]);
-        }
-        if (pending == CSTP_ERROR && !did_emsg) {
-          pending |= (THROW_ON_ERROR ? CSTP_THROW : 0);
-        } else {
-          pending |= (did_throw ? CSTP_THROW : 0);
-        }
-        pending |= did_emsg  ? CSTP_ERROR     : 0;
-        pending |= got_int   ? CSTP_INTERRUPT : 0;
-        assert(pending >= CHAR_MIN && pending <= CHAR_MAX);
-        cstack->cs_pending[cstack->cs_idx] = (char)pending;
-
-        // It's mandatory that the current exception is stored in the
-        // cstack so that it can be rethrown at the ":endtry" or be
-        // discarded if the finally clause is left by a ":continue",
-        // ":break", ":return", ":finish", error, interrupt, or another
-        // exception.  When emsg() is called for a missing ":endif" or
-        // a missing ":endwhile"/":endfor" detected here, the
-        // exception will be discarded.
-        if (did_throw && cstack->cs_exception[cstack->cs_idx] != current_exception) {
-          internal_error("ex_finally()");
-        }
+      if (pending == CSTP_ERROR && !did_emsg) {
+        pending |= (THROW_ON_ERROR ? CSTP_THROW : 0);
+      } else {
+        pending |= (did_throw ? CSTP_THROW : 0);
+      }
+      pending |= did_emsg  ? CSTP_ERROR     : 0;
+      pending |= got_int   ? CSTP_INTERRUPT : 0;
+      assert(pending >= CHAR_MIN && pending <= CHAR_MAX);
+      cstack->cs_pending[cstack->cs_idx] = (char)pending;
+
+      // It's mandatory that the current exception is stored in the
+      // cstack so that it can be rethrown at the ":endtry" or be
+      // discarded if the finally clause is left by a ":continue",
+      // ":break", ":return", ":finish", error, interrupt, or another
+      // exception.  When emsg() is called for a missing ":endif" or
+      // a missing ":endwhile"/":endfor" detected here, the
+      // exception will be discarded.
+      if (did_throw && cstack->cs_exception[cstack->cs_idx] != current_exception) {
+        internal_error("ex_finally()");
       }
-
-      // Set CSL_HAD_FINA, so do_cmdline() will reset did_emsg,
-      // got_int, and did_throw and make the finally clause active.
-      // This will happen after emsg() has been called for a missing
-      // ":endif" or a missing ":endwhile"/":endfor" detected here, so
-      // that the following finally clause will be executed even then.
-      cstack->cs_lflags |= CSL_HAD_FINA;
     }
+
+    // Set CSL_HAD_FINA, so do_cmdline() will reset did_emsg,
+    // got_int, and did_throw and make the finally clause active.
+    // This will happen after emsg() has been called for a missing
+    // ":endif" or a missing ":endwhile"/":endfor" detected here, so
+    // that the following finally clause will be executed even then.
+    cstack->cs_lflags |= CSL_HAD_FINA;
   }
 }
 
@@ -1534,165 +1533,167 @@ void ex_endtry(exarg_T *eap)
   void *rettv = NULL;
   cstack_T *const cstack = eap->cstack;
 
-  if (cstack->cs_trylevel <= 0 || cstack->cs_idx < 0) {
+  for (idx = cstack->cs_idx; idx >= 0; idx--) {
+    if (cstack->cs_flags[idx] & CSF_TRY) {
+      break;
+    }
+  }
+  if (cstack->cs_trylevel <= 0 || idx < 0) {
     eap->errmsg = _("E602: :endtry without :try");
-  } else {
-    // Don't do something after an error, interrupt or throw in the try
-    // block, catch clause, or finally clause preceding this ":endtry" or
-    // when an error or interrupt occurred after a ":continue", ":break",
-    // ":return", or ":finish" in a try block or catch clause preceding this
-    // ":endtry" or when the try block never got active (because of an
-    // inactive surrounding conditional or after an error or interrupt or
-    // throw) or when there is a surrounding conditional and it has been
-    // made inactive by a ":continue", ":break", ":return", or ":finish" in
-    // the finally clause.  The latter case need not be tested since then
-    // anything pending has already been discarded.
-    bool skip = did_emsg || got_int || did_throw || !(cstack->cs_flags[cstack->cs_idx] & CSF_TRUE);
+    return;
+  }
 
-    if (!(cstack->cs_flags[cstack->cs_idx] & CSF_TRY)) {
-      eap->errmsg = get_end_emsg(cstack);
+  // Don't do something after an error, interrupt or throw in the try
+  // block, catch clause, or finally clause preceding this ":endtry" or
+  // when an error or interrupt occurred after a ":continue", ":break",
+  // ":return", or ":finish" in a try block or catch clause preceding this
+  // ":endtry" or when the try block never got active (because of an
+  // inactive surrounding conditional or after an error or interrupt or
+  // throw) or when there is a surrounding conditional and it has been
+  // made inactive by a ":continue", ":break", ":return", or ":finish" in
+  // the finally clause.  The latter case need not be tested since then
+  // anything pending has already been discarded.
+  bool skip = did_emsg || got_int || did_throw || !(cstack->cs_flags[cstack->cs_idx] & CSF_TRUE);
 
-      // Find the matching ":try" and report what's missing.
-      idx = cstack->cs_idx;
-      do {
-        idx--;
-      } while (idx > 0 && !(cstack->cs_flags[idx] & CSF_TRY));
-      rewind_conditionals(cstack, idx, CSF_WHILE | CSF_FOR,
-                          &cstack->cs_looplevel);
-      skip = true;
+  if (!(cstack->cs_flags[cstack->cs_idx] & CSF_TRY)) {
+    eap->errmsg = get_end_emsg(cstack);
 
-      // If an exception is being thrown, discard it to prevent it from
-      // being rethrown at the end of this function.  It would be
-      // discarded by the error message, anyway.  Resets did_throw.
-      // This does not affect the script termination due to the error
-      // since "trylevel" is decremented after emsg() has been called.
-      if (did_throw) {
-        discard_current_exception();
-      }
+    // Find the matching ":try" and report what's missing.
+    rewind_conditionals(cstack, idx, CSF_WHILE | CSF_FOR,
+                        &cstack->cs_looplevel);
+    skip = true;
 
-      // report eap->errmsg, also when there already was an error
-      did_emsg = false;
-    } else {
-      idx = cstack->cs_idx;
-
-      // If we stopped with the exception currently being thrown at this
-      // try conditional since we didn't know that it doesn't have
-      // a finally clause, we need to rethrow it after closing the try
-      // conditional.
-      if (did_throw
-          && (cstack->cs_flags[idx] & CSF_TRUE)
-          && !(cstack->cs_flags[idx] & CSF_FINALLY)) {
-        rethrow = true;
-      }
+    // If an exception is being thrown, discard it to prevent it from
+    // being rethrown at the end of this function.  It would be
+    // discarded by the error message, anyway.  Resets did_throw.
+    // This does not affect the script termination due to the error
+    // since "trylevel" is decremented after emsg() has been called.
+    if (did_throw) {
+      discard_current_exception();
     }
 
-    // If there was no finally clause, show the user when debugging or
-    // a breakpoint was encountered that the end of the try conditional has
-    // been reached: display the debug prompt (if not already done).  Do
-    // this on normal control flow or when an exception was thrown, but not
-    // on an interrupt or error not converted to an exception or when
-    // a ":break", ":continue", ":return", or ":finish" is pending.  These
-    // actions are carried out immediately.
-    if ((rethrow || (!skip
-                     && !(cstack->cs_flags[idx] & CSF_FINALLY)
-                     && !cstack->cs_pending[idx]))
-        && dbg_check_skipped(eap)) {
-      // Handle a ">quit" debug command as if an interrupt had occurred
-      // before the ":endtry".  That is, throw an interrupt exception and
-      // set "skip" and "rethrow".
-      if (got_int) {
-        skip = true;
-        (void)do_intthrow(cstack);
-        // The do_intthrow() call may have reset did_throw or
-        // cstack->cs_pending[idx].
-        rethrow = false;
-        if (did_throw && !(cstack->cs_flags[idx] & CSF_FINALLY)) {
-          rethrow = true;
-        }
+    // report eap->errmsg, also when there already was an error
+    did_emsg = false;
+  } else {
+    idx = cstack->cs_idx;
+
+    // If we stopped with the exception currently being thrown at this
+    // try conditional since we didn't know that it doesn't have
+    // a finally clause, we need to rethrow it after closing the try
+    // conditional.
+    if (did_throw
+        && (cstack->cs_flags[idx] & CSF_TRUE)
+        && !(cstack->cs_flags[idx] & CSF_FINALLY)) {
+      rethrow = true;
+    }
+  }
+
+  // If there was no finally clause, show the user when debugging or
+  // a breakpoint was encountered that the end of the try conditional has
+  // been reached: display the debug prompt (if not already done).  Do
+  // this on normal control flow or when an exception was thrown, but not
+  // on an interrupt or error not converted to an exception or when
+  // a ":break", ":continue", ":return", or ":finish" is pending.  These
+  // actions are carried out immediately.
+  if ((rethrow || (!skip
+                   && !(cstack->cs_flags[idx] & CSF_FINALLY)
+                   && !cstack->cs_pending[idx]))
+      && dbg_check_skipped(eap)) {
+    // Handle a ">quit" debug command as if an interrupt had occurred
+    // before the ":endtry".  That is, throw an interrupt exception and
+    // set "skip" and "rethrow".
+    if (got_int) {
+      skip = true;
+      (void)do_intthrow(cstack);
+      // The do_intthrow() call may have reset did_throw or
+      // cstack->cs_pending[idx].
+      rethrow = false;
+      if (did_throw && !(cstack->cs_flags[idx] & CSF_FINALLY)) {
+        rethrow = true;
       }
     }
+  }
 
-    // If a ":return" is pending, we need to resume it after closing the
-    // try conditional; remember the return value.  If there was a finally
-    // clause making an exception pending, we need to rethrow it.  Make it
-    // the exception currently being thrown.
-    if (!skip) {
-      pending = cstack->cs_pending[idx];
-      cstack->cs_pending[idx] = CSTP_NONE;
-      if (pending == CSTP_RETURN) {
-        rettv = cstack->cs_rettv[idx];
-      } else if (pending & CSTP_THROW) {
-        current_exception = cstack->cs_exception[idx];
-      }
+  // If a ":return" is pending, we need to resume it after closing the
+  // try conditional; remember the return value.  If there was a finally
+  // clause making an exception pending, we need to rethrow it.  Make it
+  // the exception currently being thrown.
+  if (!skip) {
+    pending = cstack->cs_pending[idx];
+    cstack->cs_pending[idx] = CSTP_NONE;
+    if (pending == CSTP_RETURN) {
+      rettv = cstack->cs_rettv[idx];
+    } else if (pending & CSTP_THROW) {
+      current_exception = cstack->cs_exception[idx];
     }
+  }
 
-    // Discard anything pending on an error, interrupt, or throw in the
-    // finally clause.  If there was no ":finally", discard a pending
-    // ":continue", ":break", ":return", or ":finish" if an error or
-    // interrupt occurred afterwards, but before the ":endtry" was reached.
-    // If an exception was caught by the last of the catch clauses and there
-    // was no finally clause, finish the exception now.  This happens also
-    // after errors except when this ":endtry" is not within a ":try".
-    // Restore "emsg_silent" if it has been reset by this try conditional.
-    (void)cleanup_conditionals(cstack, CSF_TRY | CSF_SILENT, true);
+  // Discard anything pending on an error, interrupt, or throw in the
+  // finally clause.  If there was no ":finally", discard a pending
+  // ":continue", ":break", ":return", or ":finish" if an error or
+  // interrupt occurred afterwards, but before the ":endtry" was reached.
+  // If an exception was caught by the last of the catch clauses and there
+  // was no finally clause, finish the exception now.  This happens also
+  // after errors except when this ":endtry" is not within a ":try".
+  // Restore "emsg_silent" if it has been reset by this try conditional.
+  (void)cleanup_conditionals(cstack, CSF_TRY | CSF_SILENT, true);
 
-    if (cstack->cs_idx >= 0 && (cstack->cs_flags[cstack->cs_idx] & CSF_TRY)) {
-      cstack->cs_idx--;
-    }
-    cstack->cs_trylevel--;
+  if (cstack->cs_idx >= 0 && (cstack->cs_flags[cstack->cs_idx] & CSF_TRY)) {
+    cstack->cs_idx--;
+  }
+  cstack->cs_trylevel--;
 
-    if (!skip) {
-      report_resume_pending(pending,
-                            (pending == CSTP_RETURN) ? rettv :
-                            (pending & CSTP_THROW) ? (void *)current_exception : NULL);
-      switch (pending) {
-      case CSTP_NONE:
-        break;
+  if (!skip) {
+    report_resume_pending(pending,
+                          (pending == CSTP_RETURN) ? rettv :
+                          (pending & CSTP_THROW) ? (void *)current_exception : NULL);
+    switch (pending) {
+    case CSTP_NONE:
+      break;
 
-      // Reactivate a pending ":continue", ":break", ":return",
-      // ":finish" from the try block or a catch clause of this try
-      // conditional.  This is skipped, if there was an error in an
-      // (unskipped) conditional command or an interrupt afterwards
-      // or if the finally clause is present and executed a new error,
-      // interrupt, throw, ":continue", ":break", ":return", or
-      // ":finish".
-      case CSTP_CONTINUE:
-        ex_continue(eap);
-        break;
-      case CSTP_BREAK:
-        ex_break(eap);
-        break;
-      case CSTP_RETURN:
-        do_return(eap, false, false, rettv);
-        break;
-      case CSTP_FINISH:
-        do_finish(eap, false);
-        break;
+    // Reactivate a pending ":continue", ":break", ":return",
+    // ":finish" from the try block or a catch clause of this try
+    // conditional.  This is skipped, if there was an error in an
+    // (unskipped) conditional command or an interrupt afterwards
+    // or if the finally clause is present and executed a new error,
+    // interrupt, throw, ":continue", ":break", ":return", or
+    // ":finish".
+    case CSTP_CONTINUE:
+      ex_continue(eap);
+      break;
+    case CSTP_BREAK:
+      ex_break(eap);
+      break;
+    case CSTP_RETURN:
+      do_return(eap, false, false, rettv);
+      break;
+    case CSTP_FINISH:
+      do_finish(eap, false);
+      break;
 
-      // When the finally clause was entered due to an error,
-      // interrupt or throw (as opposed to a ":continue", ":break",
-      // ":return", or ":finish"), restore the pending values of
-      // did_emsg, got_int, and did_throw.  This is skipped, if there
-      // was a new error, interrupt, throw, ":continue", ":break",
-      // ":return", or ":finish".  in the finally clause.
-      default:
-        if (pending & CSTP_ERROR) {
-          did_emsg = true;
-        }
-        if (pending & CSTP_INTERRUPT) {
-          got_int = true;
-        }
-        if (pending & CSTP_THROW) {
-          rethrow = true;
-        }
-        break;
+    // When the finally clause was entered due to an error,
+    // interrupt or throw (as opposed to a ":continue", ":break",
+    // ":return", or ":finish"), restore the pending values of
+    // did_emsg, got_int, and did_throw.  This is skipped, if there
+    // was a new error, interrupt, throw, ":continue", ":break",
+    // ":return", or ":finish".  in the finally clause.
+    default:
+      if (pending & CSTP_ERROR) {
+        did_emsg = true;
+      }
+      if (pending & CSTP_INTERRUPT) {
+        got_int = true;
+      }
+      if (pending & CSTP_THROW) {
+        rethrow = true;
       }
+      break;
     }
+  }
 
-    if (rethrow) {
-      // Rethrow the current exception (within this cstack).
-      do_throw(cstack);
-    }
+  if (rethrow) {
+    // Rethrow the current exception (within this cstack).
+    do_throw(cstack);
   }
 }