From d174679a36d3e5fa00bed14a51e8f000b7fe679f Mon Sep 17 00:00:00 2001 From: Neil Deakin Date: Thu, 20 Oct 2022 15:37:58 +0000 Subject: [PATCH] Bug 1791033, avoid crash by computing the extension's byte length correctly and truncate the filename's extension when it is too long, r=gsvelto,jfkthame Differential Revision: https://phabricator.services.mozilla.com/D159703 --- .../exthandler/nsExternalHelperAppService.cpp | 51 ++++++++++-------- .../tests/unit/test_filename_sanitize.js | 52 +++++++++++++++++++ 2 files changed, 80 insertions(+), 23 deletions(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index 7f56f1c45212..54a8d617308c 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -3543,8 +3543,8 @@ void nsExternalHelperAppService::SanitizeFileName(nsAString& aFileName, // The number of bytes that the string would occupy if encoded in UTF-8. uint32_t bytesLength = 0; - // The length of the extension. - int32_t extensionBytesLength = 0; + // The length of the extension in bytes. + uint32_t extensionBytesLength = 0; // This algorithm iterates over each character in the string and appends it // or a replacement character if needed to outFileName. @@ -3556,9 +3556,8 @@ void nsExternalHelperAppService::SanitizeFileName(nsAString& aFileName, break; } - if (nextChar == char16_t(0)) { - continue; - } + // nulls are already stripped out above. + MOZ_ASSERT(nextChar != char16_t(0)); auto unicodeCategory = unicode::GetGeneralCategory(nextChar); if (unicodeCategory == HB_UNICODE_GENERAL_CATEGORY_CONTROL || @@ -3611,10 +3610,11 @@ void nsExternalHelperAppService::SanitizeFileName(nsAString& aFileName, if (maxBytes) { // UTF16CharEnumerator already converts surrogate pairs, so we can use // a simple computation of byte length here. - bytesLength += nextChar < 0x80 ? 1 - : nextChar < 0x800 ? 2 - : nextChar < 0x10000 ? 3 - : 4; + uint32_t charBytesLength = nextChar < 0x80 ? 1 + : nextChar < 0x800 ? 2 + : nextChar < 0x10000 ? 3 + : 4; + bytesLength += charBytesLength; if (bytesLength > maxBytes) { if (longFileNameEnd == -1) { longFileNameEnd = int32_t(outFileName.Length()); @@ -3622,11 +3622,12 @@ void nsExternalHelperAppService::SanitizeFileName(nsAString& aFileName, } // If we encounter a period, it could be the start of an extension, so - // start counting the number of bytes in the extension. + // start counting the number of bytes in the extension. If another period + // is found, start again since we want to use the last extension found. if (nextChar == u'.') { extensionBytesLength = 1; // 1 byte for the period. } else if (extensionBytesLength) { - extensionBytesLength++; + extensionBytesLength += charBytesLength; } } @@ -3638,10 +3639,10 @@ void nsExternalHelperAppService::SanitizeFileName(nsAString& aFileName, // on the filename. if (bytesLength > maxBytes && !outFileName.IsEmpty()) { // Get the sanitized extension from the filename without the dot. - nsAutoCString extension; + nsAutoString extension; int32_t dotidx = outFileName.RFind(u"."); if (dotidx != -1) { - extension = NS_ConvertUTF16toUTF8(Substring(outFileName, dotidx + 1)); + extension = Substring(outFileName, dotidx + 1); } // There are two ways in which the filename should be truncated: @@ -3661,20 +3662,24 @@ void nsExternalHelperAppService::SanitizeFileName(nsAString& aFileName, longFileNameEnd -= extensionBytesLength; if (longFileNameEnd <= 0) { // This is extremely unlikely, but if the extension is larger than the - // maximum size, just get rid of it. - outFileName.Truncate(maxBytes); + // maximum size, just get rid of it. In this case, the extension + // wouldn't have been an ordinary one we would want to preserve (such + // as .html or .png) so just truncate off the file wherever the first + // period appears. + int32_t dotidx = outFileName.Find(u"."); + outFileName.Truncate(dotidx > 0 ? dotidx : 1); } else { outFileName.Truncate(std::min(longFileNameEnd, lastNonTrimmable)); - } - // Now that the filename has been truncated, re-append the extension - // again. - if (!extension.IsEmpty()) { - if (outFileName.Last() != '.') { - outFileName.AppendLiteral("."); + // Now that the filename has been truncated, re-append the extension + // again. + if (!extension.IsEmpty()) { + if (outFileName.Last() != '.') { + outFileName.AppendLiteral("."); + } + + outFileName.Append(extension); } - - outFileName.Append(NS_ConvertUTF8toUTF16(extension)); } } } else if (lastNonTrimmable >= 0) { diff --git a/uriloader/exthandler/tests/unit/test_filename_sanitize.js b/uriloader/exthandler/tests/unit/test_filename_sanitize.js index 1eefdeff6729..aa76665d417e 100644 --- a/uriloader/exthandler/tests/unit/test_filename_sanitize.js +++ b/uriloader/exthandler/tests/unit/test_filename_sanitize.js @@ -133,6 +133,24 @@ add_task(async function validate_filename_method() { repeatStr + "sev.png" ); + // no filename, so index is used by default. + Assert.equal(checkFilename(".png", 0), "index.png"); + + // sanitization only, so index is not added, but initial period is stripped. + Assert.equal( + checkFilename(".png", mimeService.VALIDATE_SANITIZE_ONLY), + "png" + ); + + // correct .png extension is applied. + Assert.equal(checkFilename(".butterpecan.icecream", 0), "butterpecan.png"); + + // sanitization only, so extension is not modified, but initial period is stripped. + Assert.equal( + checkFilename(".butterpecan.icecream", mimeService.VALIDATE_SANITIZE_ONLY), + "butterpecan.icecream" + ); + let ext = ".fairlyLongExtension"; Assert.equal( checkFilename(repeatStr + ext, mimeService.VALIDATE_SANITIZE_ONLY), @@ -225,4 +243,38 @@ add_task(async function validate_filename_method() { AppConstants.platform == "macosx" ? "sound.mp4" : "sound.m4a", "sound.mpc" ); + + // This has a long filename with a 13 character extension. The end of the filename should be + // cropped to fit into 255 bytes. + Assert.equal( + mimeService.validateFileNameForSaving( + "라이브9.9만 시청컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 증정) + 선물용 쇼핑백 2장컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 증정) + 선물용 쇼핑백 2장24%102,000원 브랜드데이 앵콜 🎁 1.등 유산균 컬처렐 특가!", + "text/unknown", + mimeService.VALIDATE_SANITIZE_ONLY + ), + "라이브9.9만 시청컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 증정) + 선물용 쇼핑백 2장컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 .등 유산균 컬처렐 특가!", + "very long filename with extension" + ); + + // This filename has a very long extension, almost the entire filename. + Assert.equal( + mimeService.validateFileNameForSaving( + "라이브9.9만 시청컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 증정) + 선물용 쇼핑백 2장컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 증정) + 선물용 쇼핑백 2장24%102,000원 브랜드데이 앵콜 🎁 1등 유산균 컬처렐 특가!", + "text/unknown", + mimeService.VALIDATE_SANITIZE_ONLY + ), + "라이브9", + "another very long filename with long extension" + ); + + // This filename is cropped at 254 bytes. + Assert.equal( + mimeService.validateFileNameForSaving( + ".라이브99만 시청컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 증정) + 선물용 쇼핑백 2장컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 증정) + 선물용 쇼핑백 2장24%102,000원 브랜드데이 앵콜 🎁 1등 유산균 컬처렐 특가!", + "text/unknown", + mimeService.VALIDATE_SANITIZE_ONLY + ), + "라이브99만 시청컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 증정) + 선물용 쇼핑백 2장컬처렐 다이제스티브 3박스 + 3박스 더 (뚱랑이 굿즈 증정) + 선물용 쇼핑백 2장24%102,000원 브랜드데", + "very filename with extension only" + ); });