Stored Cross-site Scripting (XSS)

[l4rm4nd]

The latest sftpgo release v2.3.4 is susceptible to XSS.

docker run --name some-sftpgo -p 127.0.0.1:8888:8080 -d "drakkan/sftpgo:latest"

The "webclient" application lacks proper input validation and output sanitization during file uploads. In detail, an authenticated attacker can upload a malicious file with a filename that contains HTML or JavaScript code. Upon successful file upload, the attacker's payload is executed in the browser.

PoC filename used:

Sun'><img src=x onerror=alert('XSS-SFTPGo')>set.jpg

Note that this issue can also be exploited by using the sharing capability of sftpgo. Successful exploitation requires an attacker to craft the following exploit setup:

1. Create a new directory in the webclient, which will be shared later on.
2. Upload a malicious file with an XSS filename into the newly created directory.
3. Share the directory that holds the XSS file.
4. Send the pubshare link to a victim. Upon opening the link, the attacker's XSS payload is executed in the victim's browser.

More information to mitigate XSS can be found at: https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html

[l4rm4nd]

BTW, the "Add directory" application function is also susceptible and lacks proper sanitization.

Example payload:

<img src=x onerror=alert("XSS-SFTPGo-Directory")>

[drakkan]

Thanks, I'll try to fix, but the next time please report security related issue privately to avoid to disclose them before a fix is available.

[drakkan]

Please try a 2.3.x variant once the CI ends. The same problem may occur on other pages, I will check before releasing the next version. Thanks

[l4rm4nd]

Just had a look at the 2.3.x variant, fixes the problem at first sight. Will test some XSS variations to bypass your implemented regex.

BTW, the caption for images also lacks sanitization.

[drakkan]

Hi,

I noticed the lightbox issue, please wait a while before testing. I have some local changes that should fix the issue everywhere

diff --git a/templates/webadmin/base.html b/templates/webadmin/base.html
index 36fc8c5..a5a5aaa 100644
--- a/templates/webadmin/base.html
+++ b/templates/webadmin/base.html
@@ -265,8 +265,21 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
     <script src="{{.StaticURL}}/js/sb-admin-2.min.js"></script>
 
     <script type="text/javascript">
+        function escapeHTML(str) {
+            var div = document.createElement('div');
+            div.appendChild(document.createTextNode(str));
+            return div.innerHTML;
+        }
+
+        function unescapeHTML(escapedStr) {
+            var div = document.createElement('div');
+            div.innerHTML = escapedStr;
+            var child = div.childNodes[0];
+            return child ? child.nodeValue : '';
+        }
+
         function fixedEncodeURIComponent(str) {
-            return encodeURIComponent(str).replace(/[!'()*]/g, function (c) {
+            return encodeURIComponent(unescapeHTML(str)).replace(/[!'()*]/g, function (c) {
                 return '%' + c.charCodeAt(0).toString(16);
             });
         }
diff --git a/templates/webclient/base.html b/templates/webclient/base.html
index 49387b8..87059c9 100644
--- a/templates/webclient/base.html
+++ b/templates/webclient/base.html
@@ -219,8 +219,31 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
     <script src="{{.StaticURL}}/js/sb-admin-2.min.js"></script>
 
     <script type="text/javascript">
+        function escapeHTML(str) {
+            var div = document.createElement('div');
+            div.appendChild(document.createTextNode(str));
+            //return div.innerHTML.replace(/\"/g, '&quot;').replace(/\'/g, '&#39;');
+            return div.innerHTML;
+        }
+
+        function unescapeHTML(escapedStr) {
+            var div = document.createElement('div');
+            div.innerHTML = escapedStr;
+            var child = div.childNodes[0];
+            return child ? child.nodeValue : '';
+        }
+
+        function escapeHTMLForceSafe(str) {
+            return str
+                .replace(/&/g, '_')
+                .replace(/</g, '_')
+                .replace(/>/g, '_')
+                .replace(/\"/g, '_')
+                .replace(/\'/g, '_');
+        }
+
         function fixedEncodeURIComponent(str) {
-            return encodeURIComponent(str).replace(/[!'()*]/g, function (c) {
+            return encodeURIComponent(unescapeHTML(str)).replace(/[!'()*]/g, function (c) {
                 return '%' + c.charCodeAt(0).toString(16);
             });
         }
@@ -228,14 +251,6 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
         function replaceSlash(str){
             return str.replace(/\//g,'\u2215');
         }
-
-        var escapeHTML = function ( t ) {
-                   return t
-                           .replace( /&/g, '&amp;' )
-                           .replace( /</g, '&lt;' )
-                           .replace( />/g, '&gt;' )
-                           .replace( /"/g, '&quot;' );
-               };
     </script>
 
     <!-- Page level plugins -->
diff --git a/templates/webclient/files.html b/templates/webclient/files.html
index ee4deac..7c7d41a 100644
--- a/templates/webclient/files.html
+++ b/templates/webclient/files.html
@@ -233,13 +233,14 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
 <script type="text/javascript">
     var childReference = null;
     var checkerStarted = false;
+    var lastSelectedExtURLName;
     const childProps = new Map();
 
-    function openExternalURL(url, fileLink, fileName){
+    function openExternalURL(url, fileLink){
         if (childReference == null || childReference.closed) {
             childProps.set('link', fileLink);
             childProps.set('url', url);
-            childProps.set('file_name', fileName);
+            childProps.set('file_name', lastSelectedExtURLName);
             childReference = window.open(url, '_blank');
             if (!checkerStarted){
                 keepAlive();
@@ -432,6 +433,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
     var spinnerDone = false;
     var player;
     var playerKeepAlive;
+    var lastSelectedVideo;
 
     function shortenData(d, cutoff) {
         if ( typeof d !== 'string' ) {
@@ -446,8 +448,8 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
         return escapeHTML(shortened)+'&#8230;';
     }
 
-    function openVideoPlayer(name, url, videoType){
-        $("#video_title").text(name);
+    function openVideoPlayer(url, videoType){
+        $("#video_title").text(unescapeHTML(lastSelectedVideo));
         $('#videoModal').modal('show');
         player.src({
             type: videoType,
@@ -995,8 +997,9 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                             var title = "";
                             var cssClass = "";
                             var shortened = shortenData(data, 70);
+                            data = escapeHTML(data);
                             if (shortened != data){
-                                title = escapeHTML(data);
+                                title = data;
                                 cssClass = "ellipsis";
                             }
 
@@ -1017,7 +1020,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                 { "data": "edit_url",
                     "render": function (data, type, row) {
                         if (type === 'display') {
-                            var filename = row["name"];
+                            var filename = escapeHTML(row["name"]);
                             var extension = filename.slice((filename.lastIndexOf(".") - 1 >>> 0) + 2).toLowerCase();
                             if (data){
                                 if (extension == "csv" || extension == "bat" || CodeMirror.findModeByExtension(extension) != null){
@@ -1039,15 +1042,19 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                                     case "svg":
                                     case "ico":
                                         var view_url = row['url']+"&inline=1";
-                                        return `<a href="${view_url}" data-lightbox="image-gallery" data-title="${filename}"><i class="fas fa-eye"></i></a>`;
+                                        var title = escapeHTMLForceSafe(row["name"])
+                                        return `<a href="${view_url}" data-lightbox="image-gallery" data-title="${title}"><i class="fas fa-eye"></i></a>`;
                                     case "mp4":
                                     case "mov":
-                                        return `<a href="#" onclick="openVideoPlayer('${row["name"]}', '${row['url']}', 'video/mp4');"><i class="fas fa-eye"></i></a>`;
+                                        lastSelectedVideo = filename;
+                                        return `<a href="#" onclick="openVideoPlayer('${row['url']}', 'video/mp4');"><i class="fas fa-eye"></i></a>`;
                                     case "webm":
-                                        return `<a href="#" onclick="openVideoPlayer('${row["name"]}', '${row['url']}', 'video/webm');"><i class="fas fa-eye"></i></a>`;
+                                        lastSelectedVideo = filename;
+                                        return `<a href="#" onclick="openVideoPlayer('${row['url']}', 'video/webm');"><i class="fas fa-eye"></i></a>`;
                                     case "ogv":
                                     case "ogg":
-                                        return `<a href="#" onclick="openVideoPlayer('${row["name"]}', '${row['url']}', 'video/ogg');"><i class="fas fa-eye"></i></a>`;
+                                        lastSelectedVideo = filename;
+                                        return `<a href="#" onclick="openVideoPlayer('${row['url']}', 'video/ogg');"><i class="fas fa-eye"></i></a>`;
                                     case "pdf":
                                         if (PDFObject.supportsPDFs){
                                             var view_url = row['url'];
@@ -1065,7 +1072,8 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                         {{if .HasIntegrations}}
                         if (type === 'display') {
                             if (data){
-                                return `<a href="#" onclick="openExternalURL('${data}', '${row["ext_link"]}', '${row["name"]}');"><i class="fas fa-external-link-alt"></i></a>`;
+                                lastSelectedExtURLName = escapeHTML(row["name"]);
+                                return `<a href="#" onclick="openExternalURL('${data}', '${row["ext_link"]}');"><i class="fas fa-external-link-alt"></i></a>`;
                             }
                         }
                         {{end}}
diff --git a/templates/webclient/sharefiles.html b/templates/webclient/sharefiles.html
index 2797c7f..1370dfd 100644
--- a/templates/webclient/sharefiles.html
+++ b/templates/webclient/sharefiles.html
@@ -94,26 +94,17 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
 <script src="{{.StaticURL}}/vendor/datatables/dataTables.responsive.min.js"></script>
 <script src="{{.StaticURL}}/vendor/datatables/responsive.bootstrap4.min.js"></script>
 <script type="text/javascript">
-
-    var escapeHTML = function ( t ) {
-               return t
-                       .replace( /&/g, '&amp;' )
-                       .replace( /</g, '&lt;' )
-                       .replace( />/g, '&gt;' )
-                       .replace( /"/g, '&quot;' );
-       };
-
     function shortenData(d, cutoff) {
         if ( typeof d !== 'string' ) {
                        return d;
                }
 
                if ( d.length <= cutoff ) {
-                       return d;
+                       return escapeHTML(d);
                }
 
         var shortened = d.substr(0, cutoff-1);
-        return shortened+'&#8230;';
+        return escapeHTML(shortened)+'&#8230;';
     }
 
     function getIconForFile(filename) {
@@ -250,7 +241,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                     let response;
                     try {
                         var f = files[index];
-                        var uploadPath = '{{.UploadBaseURL}}'+fixedEncodeURIComponent("/"+f.name);
+                        var uploadPath = '{{.UploadBaseURL}}'+fixedEncodeURIComponent("/"+escapeHTML(f.name));
                         var lastModified;
                         try {
                             lastModified = f.lastModified;
@@ -384,6 +375,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                             var title = "";
                             var cssClass = "";
                             var shortened = shortenData(data, 70);
+                            data = escapeHTML(data);
                             if (shortened != data){
                                 title = escapeHTML(data);
                                 cssClass = "ellipsis";
diff --git a/templates/webclient/shareupload.html b/templates/webclient/shareupload.html
index 501e35a..d549b5a 100644
--- a/templates/webclient/shareupload.html
+++ b/templates/webclient/shareupload.html
@@ -99,7 +99,7 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
                     let response;
                     try {
                         var f = files[index];
-                        var uploadPath = '{{.UploadBasePath}}/'+fixedEncodeURIComponent(f.name);
+                        var uploadPath = '{{.UploadBasePath}}/'+fixedEncodeURIComponent(escapeHTML(f.name));
                         var lastModified;
                         try {
                             lastModified = f.lastModified;

As you can see to fix the image caption I added the escapeHTMLForceSafe method (change the image name). This seems an XSS in lightbox itself.

I'll push these changes later today after my work hours. I need some time to do proper tests. Thanks!

[drakkan]

cbef217

it should be better now. Please let me know (privately) if you find any other related issues, thanks. The image caption will be modified, for now, if it contains unsafe chars (it seems a lightbox issue but I'm not completly sure).

You can test the new changes once the CI ends

[drakkan]

Fixed also in development version

[l4rm4nd]

Hi @drakkan,

I've identified another XSS issue. I've outlined the details via email as requested.

The newly identified XSS issue is present in the newest SFTPGo release v2.3.5.

All other injection points outlined in this GitHub issue are successfully fixed in v2.3.5.