path.c: `vim_FullName()`: Fix heap overflow #5737

- Clarify documentation.
- Return `FAIL` and truncate if `fname` is too long.
- Add tests.

2 files changed, 36 insertions, 22 deletions

diff --git a/src/nvim/path.c b/src/nvim/path.c
index 6149e1ab99..3d1def8dd4 100644
--- a/src/nvim/path.c
+++ b/src/nvim/path.c
@@ -417,15 +417,11 @@ char *FullName_save(char *fname, bool force)
   }
 
   char *buf = xmalloc(MAXPATHL);
-  char *new_fname = NULL;
-  if (vim_FullName(fname, buf, MAXPATHL, force) != FAIL) {
-    new_fname = xstrdup(buf);
-  } else {
-    new_fname = xstrdup(fname);
+  if (vim_FullName(fname, buf, MAXPATHL, force) == FAIL) {
+    xfree(buf);
+    return xstrdup(fname);
   }
-  xfree(buf);
-
-  return new_fname;
+  return buf;
 }
 
 /// Saves the absolute path.
@@ -1649,30 +1645,37 @@ bool vim_isAbsName(char_u *name)
 
 /// Save absolute file name to "buf[len]".
 ///
-/// @param      fname is the filename to evaluate
-/// @param[out] buf   is the buffer for returning the absolute path for `fname`
-/// @param      len   is the length of `buf`
-/// @param      force is a flag to force expanding even if the path is absolute
+/// @param      fname filename to evaluate
+/// @param[out] buf   contains `fname` absolute path, or:
+///                   - truncated `fname` if longer than `len`
+///                   - unmodified `fname` if absolute path fails or is a URL
+/// @param      len   length of `buf`
+/// @param      force flag to force expanding even if the path is absolute
 ///
 /// @return           FAIL for failure, OK otherwise
 int vim_FullName(const char *fname, char *buf, size_t len, bool force)
   FUNC_ATTR_NONNULL_ARG(2)
 {
-  int retval = OK;
-  int url;
-
   *buf = NUL;
-  if (fname == NULL)
+  if (fname == NULL) {
     return FAIL;
+  }
 
-  url = path_with_url(fname);
-  if (!url)
-    retval = path_get_absolute_path((char_u *)fname, (char_u *)buf, len, force);
-  if (url || retval == FAIL) {
-    /* something failed; use the file name (truncate when too long) */
+  if (strlen(fname) > (len - 1)) {
+    xstrlcpy(buf, fname, len);  // truncate
+    return FAIL;
+  }
+
+  if (path_with_url(fname)) {
     xstrlcpy(buf, fname, len);
+    return OK;
   }
-  return retval;
+
+  int rv = path_get_absolute_path((char_u *)fname, (char_u *)buf, len, force);
+  if (rv == FAIL) {
+    xstrlcpy(buf, fname, len);  // something failed; use the filename
+  }
+  return rv;
 }
 
 /// Get the full resolved path for `fname`
diff --git a/test/unit/path_spec.lua b/test/unit/path_spec.lua
index 9b76834383..ccaf0228ab 100644
--- a/test/unit/path_spec.lua
+++ b/test/unit/path_spec.lua
@@ -336,6 +336,17 @@ describe('more path function', function()
       eq(FAIL, result)
     end)
 
+    it('fails safely if given length is wrong #5737', function()
+      local force_expansion = 1
+      local filename = 'foo/bar/bazzzzzzz/buz/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/a'
+      local too_short_len = 8
+      local buf = cstr(too_short_len, '')
+      local result = path.vim_FullName(filename, buf, too_short_len, force_expansion)
+      local expected = string.sub(filename, 1, (too_short_len - 1))
+      eq(expected, (ffi.string(buf)))
+      eq(FAIL, result)
+    end)
+
     it('uses the filename if the filename is a URL', function()
       local force_expansion = 1
       local filename = 'http://www.neovim.org'