From 96619a4b58a9ba3664b2588aa0960c80282f32c0 Mon Sep 17 00:00:00 2001 From: Kyle K Date: Thu, 26 Sep 2019 22:06:47 +0000 Subject: [PATCH 1/2] Don't accept fancy lookin letters. Backstory, Unified configs mean we can embed arbitrary text into the firmware, what is one source of text? Project Gutenberg! I tried attaching a snippet of the text into the config (as comments, I am a careful deviant) and ended up learning a little about how text is encoded and handled these days. Given a character with a value higher than 1 byte, the value gets cropped off, so 0x2018 would get flashed as 0x18, and would fail validation, because we'd be checking 0x2018 against 0x18 Asking Notepad++ to convert the document to ANSI was no good either, it decided to use Code page 1252, which still has fancy quotes, and when loaded into the configurator, instead of 0x92, it'd be converted to a replacement character FFFD if you want to look something up. --- locales/en/messages.json | 3 +++ src/js/tabs/firmware_flasher.js | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/locales/en/messages.json b/locales/en/messages.json index 2bff6ee2..82bbf005 100644 --- a/locales/en/messages.json +++ b/locales/en/messages.json @@ -2812,6 +2812,9 @@ "firmwareFlasherHexCorrupted": { "message": "HEX file appears to be corrupted" }, + "firmwareFlasherConfigCorrupted": { + "message": "Config file appears to be corrupted, ASCII accepted (chars 0-127)" + }, "firmwareFlasherRemoteFirmwareLoaded": { "message": "Remote Firmware loaded, ready for flashing" }, diff --git a/src/js/tabs/firmware_flasher.js b/src/js/tabs/firmware_flasher.js index f4ceefdb..2fc28445 100644 --- a/src/js/tabs/firmware_flasher.js +++ b/src/js/tabs/firmware_flasher.js @@ -587,6 +587,12 @@ TABS.firmware_flasher.initialize = function (callback) { self.flashingMessage(i18n.getMessage('firmwareFlasherFirmwareLocalLoaded', self.parsed_hex.bytes_total), self.FLASH_MESSAGE_TYPES.NEUTRAL); } } + function checkAsciiLimits(input) { + for (let i=0; i < input.length; i++) { + if (input.charCodeAt(i) > 127) { return false; } + } + return true; + } // UI Hooks $('a.load_file').click(function () { self.enableFlashing(false); @@ -641,10 +647,14 @@ TABS.firmware_flasher.initialize = function (callback) { }); } else { clearBufferedFirmware(); - self.unifiedTargetConfig = e.target.result; - self.unifiedTargetConfigName = file.name; - self.isConfigLocal = true; - flashingMessageLocal(); + if (checkAsciiLimits(e.target.result)) { + self.unifiedTargetConfig = e.target.result; + self.unifiedTargetConfigName = file.name; + self.isConfigLocal = true; + flashingMessageLocal(); + } else { + self.flashingMessage('firmwareFlasherConfigCorrupted', self.FLASH_MESSAGE_TYPES.INVALID); + } } } }; From 7133001fa27741e8148d8a213874cd6ea399a7ea Mon Sep 17 00:00:00 2001 From: Kyle K Date: Sun, 29 Sep 2019 06:13:25 +0000 Subject: [PATCH 2/2] revise limit to 255, and handle comments --- locales/en/messages.json | 7 ++++++- src/js/tabs/firmware_flasher.js | 29 +++++++++++++++++++++++------ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/locales/en/messages.json b/locales/en/messages.json index 82bbf005..b85c0f72 100644 --- a/locales/en/messages.json +++ b/locales/en/messages.json @@ -2813,7 +2813,12 @@ "message": "HEX file appears to be corrupted" }, "firmwareFlasherConfigCorrupted": { - "message": "Config file appears to be corrupted, ASCII accepted (chars 0-127)" + "message": "Config file appears to be corrupted, ASCII accepted (chars 0-255)", + "description": "shown in the progress bar at the bottom, be brief" + }, + "firmwareFlasherConfigCorruptedLogMessage": { + "message": "Config file appears to be corrupted, ASCII accepted (chars 0-255), characters outside of this range are allowed as comments", + "description": "shown in the log, more wordy" }, "firmwareFlasherRemoteFirmwareLoaded": { "message": "Remote Firmware loaded, ready for flashing" diff --git a/src/js/tabs/firmware_flasher.js b/src/js/tabs/firmware_flasher.js index 2fc28445..77a5bb29 100644 --- a/src/js/tabs/firmware_flasher.js +++ b/src/js/tabs/firmware_flasher.js @@ -587,11 +587,27 @@ TABS.firmware_flasher.initialize = function (callback) { self.flashingMessage(i18n.getMessage('firmwareFlasherFirmwareLocalLoaded', self.parsed_hex.bytes_total), self.FLASH_MESSAGE_TYPES.NEUTRAL); } } - function checkAsciiLimits(input) { + function cleanUnifiedConfigFile(input) { + let output = []; + let inComment = false; for (let i=0; i < input.length; i++) { - if (input.charCodeAt(i) > 127) { return false; } + if (input.charAt(i) == "\n" || input.charAt(i) == "\r") { + inComment = false; + } + if (input.charAt(i) == "#") { + inComment = true; + } + if (!inComment && input.charCodeAt(i) > 255) { + // Note: we're not showing this error in betaflight-configurator + throw new Error('commands are limited to characters 0-255, comments have no limitation'); + } + if (input.charCodeAt(i) > 255) { + output.push('_'); + } else { + output.push(input.charAt(i)); + } } - return true; + return output.join(''); } // UI Hooks $('a.load_file').click(function () { @@ -647,13 +663,14 @@ TABS.firmware_flasher.initialize = function (callback) { }); } else { clearBufferedFirmware(); - if (checkAsciiLimits(e.target.result)) { - self.unifiedTargetConfig = e.target.result; + try { + self.unifiedTargetConfig = cleanUnifiedConfigFile(e.target.result); self.unifiedTargetConfigName = file.name; self.isConfigLocal = true; flashingMessageLocal(); - } else { + } catch(err) { self.flashingMessage('firmwareFlasherConfigCorrupted', self.FLASH_MESSAGE_TYPES.INVALID); + GUI.log(i18n.getMessage('firmwareFlasherConfigCorruptedLogMessage')); } } }