Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/164#discussion_r191081345
--- Diff: src/common-ssh/ssh.c ---
@@ -518,6 +520,63 @@ guac_common_ssh_session*
guac_common_ssh_create_session(guac_client* client,
return NULL;
}
+ /* Check known_hosts, start by getting known_hosts file of user
running guacd */
+ struct passwd *pw = getpwuid(getuid());
+ const char *known_hosts = strcat(pw->pw_dir, "/.ssh/known_hosts");
--- End diff --
According to the documentation of `strcat()`:
> ... The strings may not overlap, and the `dest` string must have enough
space for the result. If `dest` is not large enough, program behavior is
unpredictable; buffer overruns are a favorite avenue for attacking secure
programs.
Unless we know that `pw->pw_dir` will have sufficient space for the
additional "/.ssh/known_hosts", this may not be safe as-written.
What about using a location specific to guacd like
`/etc/guacamole/ssh/known_hosts`? That would avoid inheriting known hosts from
the user running the process, and would effectively require explicit steps to
enable the checking.
---
