Vladort fixes (#3)

* Vladort fixes

Fix file right error, error 102400, error 0000 or ---- and file edit override fail when size of file is under existing file size

* Fix statError handling, remove redundant code block

os.Create will truncate an existing file, so there is no reason to remove the file first here.

Also you can't make it to that last removed point without a file existing because of the return on (now) line 114.

* Fix waterfall issue on Rmdir causing os.Chown warnings

* Default filemode for files to be 0644, otherwise use 755 for folders

* This should still respect the request flags when modifying a file

* This should still respect the request flags when modifying a file

* Fix os.OpenFile call to be used still

* I can't imaging this ever happening

* Try again for like the 5th time to do this right

* This file shouldn't be here

* Force a file truncation, don't rely on the client to send correct flags.

* Use os.Open for reads

go.mod

@@ -3,8 +3,8 @@ module github.com/pterodactyl/sftp-server
 require (
 	github.com/buger/jsonparser v0.0.0-20181023193515-52c6e1462ebd
 	github.com/kr/fs v0.1.0 // indirect
-	github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
-	github.com/pkg/errors v0.8.0 // indirect
+	github.com/patrickmn/go-cache v2.1.0+incompatible
+	github.com/pkg/errors v0.8.0
 	github.com/pkg/sftp v1.8.3
 	github.com/uber-go/zap v1.9.1 // indirect
 	go.uber.org/atomic v1.3.2 // indirect

src/server/handler.go

@@ -49,7 +49,7 @@ func (fs FileSystem) Fileread(request *sftp.Request) (io.ReaderAt, error) {
 		return nil, sftp.ErrSshFxNoSuchFile
 	}
 
-	file, err := os.OpenFile(p, os.O_RDONLY, 0644)
+	file, err := os.Open(p)
 	if err != nil {
 		logger.Get().Errorw("could not open file for reading", zap.String("source", p), zap.Error(err))
 		return nil, sftp.ErrSshFxFailure
@@ -79,7 +79,7 @@ func (fs FileSystem) Filewrite(request *sftp.Request) (io.WriterAt, error) {
 	fs.lock.Lock()
 	defer fs.lock.Unlock()
 
-	_, statErr := os.Stat(p)
+	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) {
@@ -112,8 +112,12 @@ func (fs FileSystem) Filewrite(request *sftp.Request) (io.WriterAt, error) {
 		}
 
 		return file, nil
-	} else if err != nil {
-		logger.Get().Errorw("error performing file stat", zap.String("source", p), zap.Error(err))
+	}
+
+	// If the stat error isn't about the file not existing, there is some other issue
+	// at play and we need to go ahead and bail out of the process.
+	if statErr != nil {
+		logger.Get().Errorw("error performing file stat", zap.String("source", p), zap.Error(statErr))
 		return nil, sftp.ErrSshFxFailure
 	}
 
@@ -126,9 +130,15 @@ func (fs FileSystem) Filewrite(request *sftp.Request) (io.WriterAt, error) {
 		return nil, sftp.ErrSshFxPermissionDenied
 	}
 
-	file, err := os.OpenFile(p, int(request.Flags), 0644)
+	// Not sure this would ever happen, but lets not find out.
+	if stat.IsDir() {
+		logger.Get().Warnw("attempted to open a directory for writing to", zap.String("source", p))
+		return nil, sftp.ErrSshFxOpUnsupported
+	}
+
+	file, err := os.Create(p)
 	if err != nil {
-		logger.Get().Errorw("error writing to existing file",
+		logger.Get().Errorw("error opening existing file",
 			zap.Uint32("flags", request.Flags),
 			zap.String("source", p),
 			zap.Error(err),
@@ -170,7 +180,12 @@ func (fs FileSystem) Filecmd(request *sftp.Request) error {
 
 	switch request.Method {
 	case "Setstat":
-		if err := os.Chmod(p, request.Attributes().FileMode()); err != nil {
+		var mode os.FileMode = 0644
+		if request.Attributes().FileMode().IsDir() {
+			mode = 0755
+		}
+
+		if err := os.Chmod(p, mode); err != nil {
 			logger.Get().Errorw("failed to perform setstat", zap.Error(err))
 			return sftp.ErrSshFxFailure
 		}
@@ -200,7 +215,7 @@ func (fs FileSystem) Filecmd(request *sftp.Request) error {
 			return sftp.ErrSshFxFailure
 		}
 
-		break
+		return sftp.ErrSshFxOk
 	case "Mkdir":
 		if !fs.can("create-files") {
 			return sftp.ErrSshFxPermissionDenied
@@ -248,7 +263,8 @@ func (fs FileSystem) Filecmd(request *sftp.Request) error {
 	}
 
 	// Not failing here is intentional. We still made the file, it is just owned incorrectly
-	// and will likely cause some issues.
+	// and will likely cause some issues. There is no logical check for if the file was removed
+	// because both of those cases (Rmdir, Remove) have an explicit return rather than break.
 	if err := os.Chown(fileLocation, fs.User.Uid, fs.User.Gid); err != nil {
 		logger.Get().Warnw("error chowning file", zap.String("file", fileLocation), zap.Error(err))
 	}
@@ -436,4 +452,4 @@ func (fs FileSystem) directorySize(dir string) int64 {
 	wg.Wait()
 
 	return size
-}
+}