From 719f6077ab4c0ba569ae2f5a0a601d30c6d49cce Mon Sep 17 00:00:00 2001
From: Nicola Murino <nicola.murino@gmail.com>
Date: Tue, 28 Jun 2022 22:08:16 +0200
Subject: [PATCH] don't expose underlying errors to clients

log them and return a generic failure

Fixes #896

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>
---
 common/connection.go      | 4 +++-
 common/connection_test.go | 2 --
 sftpd/internal_test.go    | 2 --
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/common/connection.go b/common/connection.go
index 29658c3a..09230d39 100644
--- a/common/connection.go
+++ b/common/connection.go
@@ -1345,9 +1345,11 @@ func (c *BaseConnection) GetGenericError(err error) error {
 		}
 		if err != nil {
 			if e, ok := err.(*os.PathError); ok {
+				c.Log(logger.LevelError, "generic path error: %+v", e)
 				return fmt.Errorf("%w: %v %v", sftp.ErrSSHFxFailure, e.Op, e.Err.Error())
 			}
-			return fmt.Errorf("%w: %v", sftp.ErrSSHFxFailure, err.Error())
+			c.Log(logger.LevelError, "generic error: %+v", err)
+			return fmt.Errorf("%w: %v", sftp.ErrSSHFxFailure, ErrGenericFailure.Error())
 		}
 		return sftp.ErrSSHFxFailure
 	default:
diff --git a/common/connection_test.go b/common/connection_test.go
index 7a2c3138..7b3761ec 100644
--- a/common/connection_test.go
+++ b/common/connection_test.go
@@ -325,14 +325,12 @@ func TestErrorsMapping(t *testing.T) {
 		err = conn.GetFsError(fs, os.ErrClosed)
 		if protocol == ProtocolSFTP {
 			assert.ErrorIs(t, err, sftp.ErrSSHFxFailure)
-			assert.Contains(t, err.Error(), os.ErrClosed.Error())
 		} else {
 			assert.EqualError(t, err, ErrGenericFailure.Error())
 		}
 		err = conn.GetFsError(fs, ErrPermissionDenied)
 		if protocol == ProtocolSFTP {
 			assert.ErrorIs(t, err, sftp.ErrSSHFxFailure)
-			assert.Contains(t, err.Error(), ErrPermissionDenied.Error())
 		} else {
 			assert.EqualError(t, err, ErrPermissionDenied.Error())
 		}
diff --git a/sftpd/internal_test.go b/sftpd/internal_test.go
index d55a02b7..764d5e4a 100644
--- a/sftpd/internal_test.go
+++ b/sftpd/internal_test.go
@@ -176,7 +176,6 @@ func TestUploadResumeInvalidOffset(t *testing.T) {
 	err = transfer.Close()
 	if assert.Error(t, err) {
 		assert.ErrorIs(t, err, sftp.ErrSSHFxFailure)
-		assert.Contains(t, err.Error(), "invalid write offset")
 	}
 
 	err = os.Remove(testfile)
@@ -278,7 +277,6 @@ func TestTransferCancelFn(t *testing.T) {
 	err = transfer.Close()
 	if assert.Error(t, err) {
 		assert.ErrorIs(t, err, sftp.ErrSSHFxFailure)
-		assert.Contains(t, err.Error(), errFake.Error())
 	}
 	if assert.Error(t, transfer.ErrTransfer) {
 		assert.EqualError(t, transfer.ErrTransfer, errFake.Error())