[PATCH] daemon: detect and reject too-long paths

[Jeff King]

When we are checking the path via path_ok(), we use some
fixed PATH_MAX buffers. We write into them via snprintf(),
so there's no possibility of overflow, but it does mean we
may silently truncate the path, leading to potentially
confusing errors when the partial path does not exist.

We're better off to reject the path explicitly.

Signed-off-by: Jeff King <p...@peff.net>
---
Another option would be to switch to strbufs here. That potentially
introduces cases where a client can convince us to just keep allocating
memory, but I don't think so in practice; the paths and interpolated
data items all have to come in 64K pkt-lines, which places a hard
limit. This is a much more minimal change, though, and I don't hear
anybody complaining about the inability to use large paths.

 daemon.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/daemon.c b/daemon.c
index 425aad0507..ff0fa583b0 100644
--- a/daemon.c
+++ b/daemon.c
@@ -160,6 +160,7 @@ static const char *path_ok(const char *directory, struct 
hostinfo *hi)
 {
        static char rpath[PATH_MAX];
        static char interp_path[PATH_MAX];
+       size_t rlen;
        const char *path;
        const char *dir;
 
@@ -187,8 +188,12 @@ static const char *path_ok(const char *directory, struct 
hostinfo *hi)
                        namlen = slash - dir;
                        restlen -= namlen;
                        loginfo("userpath <%s>, request <%s>, namlen %d, 
restlen %d, slash <%s>", user_path, dir, namlen, restlen, slash);
-                       snprintf(rpath, PATH_MAX, "%.*s/%s%.*s",
-                                namlen, dir, user_path, restlen, slash);
+                       rlen = snprintf(rpath, sizeof(rpath), "%.*s/%s%.*s",
+                                       namlen, dir, user_path, restlen, slash);
+                       if (rlen >= sizeof(rpath)) {
+                               logerror("user-path too large: %s", rpath);
+                               return NULL;
+                       }
                        dir = rpath;
                }
        }
@@ -207,7 +212,15 @@ static const char *path_ok(const char *directory, struct 
hostinfo *hi)
 
                strbuf_expand(&expanded_path, interpolated_path,
                              expand_path, &context);
-               strlcpy(interp_path, expanded_path.buf, PATH_MAX);
+
+               rlen = strlcpy(interp_path, expanded_path.buf,
+                              sizeof(interp_path));
+               if (rlen >= sizeof(interp_path)) {
+                       logerror("interpolated path too large: %s",
+                                interp_path);
+                       return NULL;
+               }
+
                strbuf_release(&expanded_path);
                loginfo("Interpolated dir '%s'", interp_path);
 
@@ -219,7 +232,11 @@ static const char *path_ok(const char *directory, struct 
hostinfo *hi)
                        logerror("'%s': Non-absolute path denied (base-path 
active)", dir);
                        return NULL;
                }
-               snprintf(rpath, PATH_MAX, "%s%s", base_path, dir);
+               rlen = snprintf(rpath, sizeof(rpath), "%s%s", base_path, dir);
+               if (rlen >= sizeof(rpath)) {
+                       logerror("base-path too large: %s", rpath);
+                       return NULL;
+               }
                dir = rpath;
        }
 
-- 
2.10.1.776.ge0e381e