Fix/feedback preview xss

frontend/express/app.js

@@ -533,7 +533,8 @@ Promise.all([plugins.dbConnection(countlyConfig), plugins.dbConnection("countly_
                                 'Date': new Date().toUTCString(),
                                 'Last-Modified': stats.mtime.toUTCString(),
                                 'Content-Type': 'image/png',
-                                'Content-Length': stats.size
+                                'Content-Length': stats.size,
+                                'X-Content-Type-Options': 'nosniff'
                             });
                             stream.pipe(res);
                         }
@@ -581,7 +582,8 @@ Promise.all([plugins.dbConnection(countlyConfig), plugins.dbConnection("countly_
                                 'Date': new Date().toUTCString(),
                                 'Last-Modified': stats.mtime.toUTCString(),
                                 'Content-Type': 'image/png',
-                                'Content-Length': stats.size
+                                'Content-Length': stats.size,
+                                'X-Content-Type-Options': 'nosniff'
                             });
                             stream.pipe(res);
                         }
@@ -620,7 +622,8 @@ Promise.all([plugins.dbConnection(countlyConfig), plugins.dbConnection("countly_
                     'Date': new Date().toUTCString(),
                     'Last-Modified': stats.mtime.toUTCString(),
                     'Content-Type': 'image/png',
-                    'Content-Length': stats.size
+                    'Content-Length': stats.size,
+                    'X-Content-Type-Options': 'nosniff'
                 });
                 stream.pipe(res);
             });

plugins/dashboards/frontend/app.js

@@ -53,7 +53,8 @@ var log = common.log('dashboards:frontend');
                         'Date': new Date().toUTCString(),
                         'Last-Modified': stats.mtime.toUTCString(),
                         'Content-Type': 'image/png',
-                        'Content-Length': stats.size
+                        'Content-Length': stats.size,
+                        'X-Content-Type-Options': 'nosniff'
                     });
                     stream.pipe(res);
                 });

plugins/star-rating/api/api.js

@@ -5,8 +5,9 @@ var exported = {},
     log = common.log('star-rating:api'),
     countlyCommon = require('../../../api/lib/countly.common.js'),
     plugins = require('../../pluginManager.js'),
-    { validateCreate, validateRead, validateUpdate, validateDelete } = require('../../../api/utils/rights.js'),
-    countlyFs = require('../../../api/utils/countlyFs.js');
+    { validateCreate, validateRead, validateUpdate, validateDelete, validateGlobalAdmin, validateAppAdmin } = require('../../../api/utils/rights.js'),
+    countlyFs = require('../../../api/utils/countlyFs.js'),
+    imageUtils = require('./image-utils.js');
 var fetch = require('../../../api/parts/data/fetch.js');
 var ejs = require("ejs"),
     fs = require('fs'),
@@ -243,6 +244,16 @@ function create_upload_dir(callback) {
     });
 }
 
+// Map sniffed image MIME → file extension to use on disk. Determines
+// the saved filename and the URL component returned to the caller.
+// Restricted to png/jpeg/gif to preserve the original /i/feedback/logo
+// contract (no widening to webp here).
+var SNIFFED_TYPE_TO_EXT = {
+    "image/png": "png",
+    "image/jpeg": "jpg",
+    "image/gif": "gif"
+};
+
 /**
 * Used for file upload
 * @param {object} myfile - file object(if empty - returns)
@@ -255,48 +266,38 @@ function uploadFile(myfile, id, callback) {
         return;
     }
     var tmp_path = myfile.path;
-    var type = myfile.type;
-    myfile.name = myfile.name || "png";
-    if (type !== "image/png" && type !== "image/gif" && type !== "image/jpeg") {
-        fs.unlink(tmp_path, function() { });
-        callback("Invalid image format. Must be png or jpeg");
-        return;
-    }
-
-    var allowedExtensions = ["gif", "jpeg", "jpg", "png"];
-    var ext = myfile.name.split(".");
-    ext = ext[ext.length - 1];
-
-    if (allowedExtensions.indexOf(ext) === -1) {
-        callback("Invalid file extension. Must be .png, .jpg, .gif or .jpeg");
-        return;
-    }
 
     create_upload_dir(function() {
         fs.readFile(tmp_path, (err, data) => {
-            if (err) {
+            if (err || !data) {
+                fs.unlink(tmp_path, function() { });
                 callback("Failed to upload image");
                 return;
             }
-            //convert file to data
-            if (data) {
-                try {
-                    var pp = path.resolve(__dirname, './../images/' + id + "." + ext);
-                    countlyFs.saveData("star-rating", pp, data, { id: "" + id + "." + ext, writeMode: "overwrite" }, function(err3) {
-                        if (err3) {
-                            callback("Failed to upload image");
-                        }
-                        else {
-                            fs.unlink(tmp_path, function() { });
-                            callback(true, id + "." + ext);
-                        }
-                    });
-                }
-                catch (SyntaxError) {
-                    callback("Failed to upload image");
-                }
+            // Detect format from magic bytes; never trust myfile.type or
+            // myfile.name. Anything not in the allowlist (png/jpeg/gif)
+            // is rejected.
+            var detectedType = imageUtils.sniffImageType(data);
+            var detectedExt = detectedType && SNIFFED_TYPE_TO_EXT[detectedType];
+            if (!detectedExt) {
+                fs.unlink(tmp_path, function() { });
+                callback("Invalid image format. Must be png, jpeg, or gif");
+                return;
             }
-            else {
+            try {
+                var pp = path.resolve(__dirname, './../images/' + id + "." + detectedExt);
+                countlyFs.saveData("star-rating", pp, data, { id: "" + id + "." + detectedExt, writeMode: "overwrite" }, function(err3) {
+                    fs.unlink(tmp_path, function() { });
+                    if (err3) {
+                        callback("Failed to upload image");
+                    }
+                    else {
+                        callback(true, id + "." + detectedExt);
+                    }
+                });
+            }
+            catch (SyntaxError) {
+                fs.unlink(tmp_path, function() { });
                 callback("Failed to upload image");
             }
         });
@@ -371,36 +372,39 @@ function uploadFile(myfile, id, callback) {
     function uploadFeedbackFile(myname, myfile) {
         return new Promise(function(resolve, reject) {
             var tmp_path = myfile.path;
-            var type = myfile.type;
             if (myfile.size > 1.5 * 1024 * 1024) {
                 fs.unlink(tmp_path, function() {});
                 reject(Error("feedback.image-error"));
             }
             else {
                 fs.readFile(tmp_path, (err, data) => {
-                    if (err) {
+                    if (err || !data) {
+                        fs.unlink(tmp_path, function() {});
                         reject(Error("feedback.imagee-error"));
+                        return;
                     }
-                    //convert file to data
-                    if (data) {
-                        try {
-                            var data_uri_prefix = "data:" + type + ";base64,";
-                            var buf = Buffer.from(data);
-                            var image = buf.toString('base64');
-                            image = data_uri_prefix + image;
-                            countlyFs.gridfs.saveData("feedback", myname, image, {id: myname, writeMode: "overwrite"}, function(err2) {
-                                fs.unlink(tmp_path, function() {});
-                                if (err2) {
-                                    return reject(err2);
-                                }
-                                resolve();
-                            });
-                        }
-                        catch (SyntaxError) {
-                            reject(Error("feedback.imagee-error"));
-                        }
+                    var buf = Buffer.from(data);
+                    // Detect MIME from the actual bytes; never trust myfile.type.
+                    // Anything that isn't a recognized safe raster image is rejected.
+                    var detectedType = imageUtils.sniffImageType(buf);
+                    if (!detectedType) {
+                        fs.unlink(tmp_path, function() {});
+                        reject(Error("feedback.imagef-error"));
+                        return;
                     }
-                    else {
+                    try {
+                        var data_uri_prefix = "data:" + detectedType + ";base64,";
+                        var image = data_uri_prefix + buf.toString('base64');
+                        countlyFs.gridfs.saveData("feedback", myname, image, {id: myname, writeMode: "overwrite"}, function(err2) {
+                            fs.unlink(tmp_path, function() {});
+                            if (err2) {
+                                return reject(err2);
+                            }
+                            resolve();
+                        });
+                    }
+                    catch (SyntaxError) {
+                        fs.unlink(tmp_path, function() {});
                         reject(Error("feedback.imagee-error"));
                     }
                 });
@@ -430,37 +434,64 @@ function uploadFile(myfile, id, callback) {
      * }
     */
     plugins.register("/i/feedback/upload", function(ob) {
-        // do not respond if this isn't feedback fetch request 
+        // do not respond if this isn't feedback fetch request
         // or surveys plugin enabled
         if (surveysEnabled) {
             return false;
         }
 
         var params = ob.params;
-        validateUpdate(params, "global_plugins", function() {
-            var images = ["feedback_logo"];
-            var flag = 0;
-            if (params.files) {
-                for (let i = 0; i < images.length; i++) {
-                    if (params.files[images[i]]) {
-                        flag = 1;
-                        uploadFeedbackFile(images[i], params.files[images[i]]).then(function() {
-                            common.returnOutput(params, {"result": "Success"});
-                        }, function(err) {
-                            common.returnMessage(params, 400, err.message);
-                        });
-                        break;
-                    }
-                }
-                if (flag === 0) {
-                    uploadFeedbackFile(params.qstring.name, params.files.file).then(function() {
-                        common.returnOutput(params, {"result": "Success"});
-                    }, function(err) {
-                        common.returnMessage(params, 400, err.message);
-                    });
-                }
-            }
-        });
+        if (!params.files) {
+            common.returnMessage(params, 400, "feedback.imagee-error");
+            return true;
+        }
+
+        // Two upload modes:
+        //   1. Global feedback logo: file posted under field "feedback_logo",
+        //      stored under id "feedback_logo". Modifies a global plugin
+        //      setting and therefore requires actual global admin.
+        //   2. Per-app feedback logo: file posted under field "file" with
+        //      ?name=feedback_logo<24-char-hex-app-id>. The target app id is
+        //      decoded from the name itself, NOT taken from qstring.app_id,
+        //      so an admin of one app can't plant a logo for another app.
+        //
+        // Any other name shape is rejected.
+        var globalUpload = !!params.files.feedback_logo;
+        var fileObj = globalUpload ? params.files.feedback_logo : params.files.file;
+        var requestedName = globalUpload ? "feedback_logo" : (params.qstring && params.qstring.name);
+
+        var parsed = imageUtils.parseFeedbackLogoName(requestedName);
+        if (!parsed.valid || !fileObj) {
+            // Reuse the existing localized "invalid format" key rather than
+            // adding a new key that would need translating across 25+ locale
+            // files. From the user's perspective both conditions (bad name
+            // shape, missing file) are "your upload was malformed, try again".
+            common.returnMessage(params, 400, "feedback.imagef-error");
+            return true;
+        }
+
+        /**
+         * Run the actual file upload after auth has passed.
+         * @returns {void}
+         */
+        function performUpload() {
+            uploadFeedbackFile(requestedName, fileObj).then(function() {
+                common.returnOutput(params, {"result": "Success"});
+            }, function(err) {
+                common.returnMessage(params, 400, err.message);
+            });
+        }
+
+        if (parsed.isGlobal) {
+            validateGlobalAdmin(params, performUpload);
+        }
+        else {
+            // Pin app_id to the value embedded in the upload name so the
+            // app-admin check can't be satisfied with admin rights on a
+            // different app.
+            params.qstring.app_id = parsed.appId;
+            validateAppAdmin(params, performUpload);
+        }
         return true;
     });
     /*

plugins/star-rating/api/image-utils.js

@@ -0,0 +1,59 @@
+/**
+ * Detect image MIME type by inspecting magic bytes. Does NOT trust
+ * any client-supplied or stored MIME string. Returns one of the
+ * allowlisted image MIME types, or null if the buffer is not a
+ * recognized safe image format.
+ * @param {Buffer} buf - file content
+ * @returns {string|null} MIME type or null if not a recognized image
+ */
+function sniffImageType(buf) {
+    if (!buf || buf.length < 12) {
+        return null;
+    }
+    // PNG: 89 50 4E 47 0D 0A 1A 0A
+    if (buf[0] === 0x89 && buf[1] === 0x50 && buf[2] === 0x4E && buf[3] === 0x47
+        && buf[4] === 0x0D && buf[5] === 0x0A && buf[6] === 0x1A && buf[7] === 0x0A) {
+        return 'image/png';
+    }
+    // JPEG: FF D8 FF
+    if (buf[0] === 0xFF && buf[1] === 0xD8 && buf[2] === 0xFF) {
+        return 'image/jpeg';
+    }
+    // GIF87a / GIF89a: 47 49 46 38 (37|39) 61
+    if (buf[0] === 0x47 && buf[1] === 0x49 && buf[2] === 0x46 && buf[3] === 0x38
+        && (buf[4] === 0x37 || buf[4] === 0x39) && buf[5] === 0x61) {
+        return 'image/gif';
+    }
+    // WebP: "RIFF" .... "WEBP"
+    if (buf[0] === 0x52 && buf[1] === 0x49 && buf[2] === 0x46 && buf[3] === 0x46
+        && buf[8] === 0x57 && buf[9] === 0x45 && buf[10] === 0x42 && buf[11] === 0x50) {
+        return 'image/webp';
+    }
+    return null;
+}
+
+// Allowed feedback logo names: the literal global "feedback_logo" or
+// "feedback_logo<24-char-hex-app-id>" for per-app logos.
+var FEEDBACK_LOGO_NAME_RE = /^feedback_logo([a-f0-9]{24})?$/;
+
+/**
+ * Validate a feedback logo name and, if it's a per-app logo, return
+ * the app id encoded in it.
+ * @param {string} name - candidate name
+ * @returns {object} parse result with `valid`, `isGlobal`, and `appId` fields
+ */
+function parseFeedbackLogoName(name) {
+    if (typeof name !== 'string') {
+        return {valid: false, isGlobal: false, appId: null};
+    }
+    var m = FEEDBACK_LOGO_NAME_RE.exec(name);
+    if (!m) {
+        return {valid: false, isGlobal: false, appId: null};
+    }
+    return {valid: true, isGlobal: !m[1], appId: m[1] || null};
+}
+
+module.exports = {
+    sniffImageType: sniffImageType,
+    parseFeedbackLogoName: parseFeedbackLogoName
+};