sftpd: explicitly disallow some commands on root directory

It was possible to remove an empty root dir or create a symlink to it.
We now return a Permission Denied error if we detect an attempt to remove,
renaming or symlinking the root directory
This commit is contained in:
Nicola Murino 2019-12-25 23:37:37 +01:00
parent 489101668c
commit ae812e55af
2 changed files with 45 additions and 6 deletions

View file

@ -111,8 +111,6 @@ func (c Connection) Filewrite(request *sftp.Request) (io.WriterAt, error) {
defer c.lock.Unlock()
stat, statErr := os.Stat(p)
// If the file doesn't exist we need to create it, as well as the directory pathway
// leading up to where that file will be created.
if os.IsNotExist(statErr) {
if !c.User.HasPerm(dataprovider.PermUpload, filepath.Dir(p)) {
return nil, sftp.ErrSSHFxPermissionDenied
@ -147,12 +145,13 @@ func (c Connection) Filecmd(request *sftp.Request) error {
if err != nil {
return getSFTPErrorFromOSError(err)
}
target, err := c.getSFTPCmdTargetPath(request.Target)
if err != nil {
return err
}
isHomeDir := c.User.GetRelativePath(p) == "/"
c.Log(logger.LevelDebug, logSender, "new cmd, method: %v, sourcePath: %#v, targetPath: %#v", request.Method,
p, target)
@ -160,12 +159,19 @@ func (c Connection) Filecmd(request *sftp.Request) error {
case "Setstat":
return c.handleSFTPSetstat(p, request)
case "Rename":
if isHomeDir {
c.Log(logger.LevelWarn, logSender, "renaming root dir is not allowed")
return sftp.ErrSSHFxPermissionDenied
}
if err = c.handleSFTPRename(p, target); err != nil {
return err
}
break
case "Rmdir":
if isHomeDir {
c.Log(logger.LevelWarn, logSender, "removing root dir is not allowed")
return sftp.ErrSSHFxPermissionDenied
}
return c.handleSFTPRmdir(p)
case "Mkdir":
@ -173,13 +179,15 @@ func (c Connection) Filecmd(request *sftp.Request) error {
if err != nil {
return err
}
break
case "Symlink":
if isHomeDir {
c.Log(logger.LevelWarn, logSender, "symlinking root dir is not allowed")
return sftp.ErrSSHFxPermissionDenied
}
if err = c.handleSFTPSymlink(p, target); err != nil {
return err
}
break
case "Remove":
return c.handleSFTPRemove(p)

View file

@ -2406,6 +2406,37 @@ func TestPermsSubDirsCommands(t *testing.T) {
os.RemoveAll(user.GetHomeDir())
}
func TestRootDirCommands(t *testing.T) {
usePubKey := true
u := getTestUser(usePubKey)
u.Permissions["/"] = []string{dataprovider.PermAny}
u.Permissions["/subdir"] = []string{dataprovider.PermDownload, dataprovider.PermUpload}
user, _, err := httpd.AddUser(u, http.StatusOK)
if err != nil {
t.Errorf("unable to add user: %v", err)
}
client, err := getSftpClient(user, usePubKey)
if err != nil {
t.Errorf("unable to create sftp client: %v", err)
} else {
defer client.Close()
err = client.Rename("/", "rootdir")
if !strings.Contains(err.Error(), "Permission Denied") {
t.Errorf("unexpected error renaming root dir: %v", err)
}
err = client.Symlink("/", "rootdir")
if !strings.Contains(err.Error(), "Permission Denied") {
t.Errorf("unexpected error symlinking root dir: %v", err)
}
err = client.RemoveDirectory("/")
if !strings.Contains(err.Error(), "Permission Denied") {
t.Errorf("unexpected error removing root dir: %v", err)
}
}
httpd.RemoveUser(user, http.StatusOK)
os.RemoveAll(user.GetHomeDir())
}
func TestRelativePaths(t *testing.T) {
user := getTestUser(true)
path := filepath.Join(user.HomeDir, "/")