Hello,
On 12/6/24 5:11 PM, Christopher Bii wrote:
Hello,
It is hinted in the configuration files that an attacker could gain access
to arbitrary folders by guessing symlink paths that match exported dirs,
but this is not the case. They can get access to the root export with
certainty by simply symlinking to "../../../../../../../", which will
always return "/".
This is due to realpath() being called in the main thread which isn't
chrooted, concatenating the result with the export root to create the
export entry's final absolute path which the kernel then exports.
PS: I already sent this patch to the mailing list about the same subject
but it was poorly formatted. Changes were merged into a single commit. I
have broken it up into smaller commits and made the patch into a single
thread. Pardon the mistake, first contribution.
First of all thank you this contribution... but :-)
the patch makes an assumption that is incorrect.
An export directory has to exist when exported.
The point being... even though the export does not
exist when exported... it can in the future due
to mounting races. This is a change NeilBrown
made a few years back...
Which means the following fails which does not
with the original code
exportfs -ua
exportfs -vi *:/not_exist
exportfs: Failed to stat /not_exist: No such file or directory
(which is a warning, not an error meaning /not_exist is still exported)
With these patches this valid export fails
exportfs -vi *:/export_test fails with
exportfs: nfsd_realpath(): unable to resolve path /not_exist
because /not_exist is exported (aka in /var/lib/nfs/etab)
so the failure is not correct because /not_exist could
exist in the future.
Thank you Yongcheng Yang for this test which points
out the problem.
Here is part of the patch that needs work
in getexportent:
/* resolve symlinks */
- if (realpath(ee.e_path, rpath) != NULL) {
- rpath[sizeof (rpath) - 1] = '\0';
- strncpy(ee.e_path, rpath, sizeof (ee.e_path) - 1);
- ee.e_path[sizeof (ee.e_path) - 1] = '\0';
- }
+ if (nfsd_realpath(ee.e_path, rpath) == NULL) {
+ xlog(L_ERROR, "nfsd_realpath(): unable to resolve path
%s", ee.e_path);
+ goto out;
+ };
the current code ignores the realpath() failure... your patch does not.
I would like to fix the symlink vulnerability you have pointed
out... but stay with the assumptions of the original code.
I'll be more than willing to work with you to make this happen!
steved.
Thanks
Christopher Bii (5):
nfsd_path.h - nfsd_path.c: - Configured export rootdir must now be an
absolute path - Rootdir is into a global variable what will also be
used to retrieve it later on - nfsd_path_nfsd_rootdir(void) is
simplified with nfsd_path_rootdir which returns the global var
rather than reprobing config for rootdir entry
nfsd_path.c: - Simplification of nfsd_path_strip_root(char*)
nfsd_path.h - nfsd_path.c: - nfsd_path_prepend_dir(const char*, const
char*) -> nfsd_path_prepend_root(const char*)
NFS export symlink vulnerability fix - Replaced dangerous use of
realpath within support/nfs/export.c with nfsd_realpath variant
that is executed within the chrooted thread rather than main
thread. - Implemented nfsd_path.h methods to work securely within
chrooted thread using nfsd_run_task() helper
support/nfs/exports.c - Small changes
support/export/export.c | 17 +-
support/include/nfsd_path.h | 9 +-
support/misc/nfsd_path.c | 362 ++++++++++++------------------------
support/nfs/exports.c | 49 ++---
4 files changed, 151 insertions(+), 286 deletions(-)
