From 81c14351eedc66ea39e5412a8b0b5214969da241 Mon Sep 17 00:00:00 2001 From: Ludy Date: Thu, 22 Jan 2026 20:48:49 +0100 Subject: [PATCH] =?UTF-8?q?fix(common):=20=F0=9F=9B=A1=EF=B8=8FCWE-681=20&?= =?UTF-8?q?=20CWE-197=20eliminate=20tainted=20numeric=20casts=20in=20size?= =?UTF-8?q?=20parsing=20by=20using=20BigDecimal=20with=20range=20guards=20?= =?UTF-8?q?(#5521)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description of Changes This pull request refactors and improves the logic for converting human-readable size strings (like "10MB", "2.5GB") to bytes in the `GeneralUtils` utility class. The main enhancement is switching from imprecise floating-point arithmetic to `BigDecimal` for more accurate and robust conversions, and centralizing the conversion logic to reduce code duplication and improve maintainability. **Improvements to size conversion logic:** * Replaced all floating-point arithmetic in `convertSizeToBytes` with `BigDecimal` operations to ensure precision and to handle large values more safely. * Introduced a new private method `toBytes(BigDecimal value, int powerOf1024)` to centralize and standardize the conversion from size units to bytes, including error handling for negative and excessively large values. * Added constants `KIB` and `LONG_MAX_DECIMAL` for improved readability and maintainability of size calculations. * Added a helper method `parseSizeValue(String value)` to consistently parse size values as `BigDecimal`. * Updated imports to include `BigDecimal` and `RoundingMode` for the new conversion logic. --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing) for more details. --- .../software/common/util/GeneralUtils.java | 67 ++++++++++++------- .../util/GeneralUtilsAdditionalTest.java | 8 +++ 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java b/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java index 5f9d2a14f..0d1e32609 100644 --- a/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java +++ b/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java @@ -4,6 +4,8 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.math.BigDecimal; +import java.math.RoundingMode; import java.net.*; import java.nio.charset.StandardCharsets; import java.nio.file.*; @@ -38,6 +40,10 @@ public class GeneralUtils { */ private static final int MAX_DNS_ADDRESSES = 20; + // Constants for size conversion + private static final BigDecimal KIB = BigDecimal.valueOf(1024L); + private static final BigDecimal LONG_MAX_DECIMAL = BigDecimal.valueOf(Long.MAX_VALUE); + private final Set DEFAULT_VALID_SCRIPTS = Set.of("png_to_webp.py", "split_photos.py"); private final Set DEFAULT_VALID_PIPELINE = Set.of( @@ -539,39 +545,26 @@ public class GeneralUtils { try { if (sizeStr.endsWith("TB")) { - return (long) - (Double.parseDouble(sizeStr.substring(0, sizeStr.length() - 2)) - * 1024L - * 1024L - * 1024L - * 1024L); + return toBytes(parseSizeValue(sizeStr.substring(0, sizeStr.length() - 2)), 4); } else if (sizeStr.endsWith("GB")) { - return (long) - (Double.parseDouble(sizeStr.substring(0, sizeStr.length() - 2)) - * 1024L - * 1024L - * 1024L); + return toBytes(parseSizeValue(sizeStr.substring(0, sizeStr.length() - 2)), 3); } else if (sizeStr.endsWith("MB")) { - return (long) - (Double.parseDouble(sizeStr.substring(0, sizeStr.length() - 2)) - * 1024L - * 1024L); + return toBytes(parseSizeValue(sizeStr.substring(0, sizeStr.length() - 2)), 2); } else if (sizeStr.endsWith("KB")) { - return (long) - (Double.parseDouble(sizeStr.substring(0, sizeStr.length() - 2)) * 1024L); + return toBytes(parseSizeValue(sizeStr.substring(0, sizeStr.length() - 2)), 1); } else if (!sizeStr.isEmpty() && sizeStr.charAt(sizeStr.length() - 1) == 'B') { - return Long.parseLong(sizeStr.substring(0, sizeStr.length() - 1)); + return toBytes(parseSizeValue(sizeStr.substring(0, sizeStr.length() - 1)), 0); } else { // Use provided default unit or fall back to MB String unit = defaultUnit != null ? defaultUnit.toUpperCase(Locale.ROOT) : "MB"; - double value = Double.parseDouble(sizeStr); + BigDecimal value = parseSizeValue(sizeStr); return switch (unit) { - case "TB" -> (long) (value * 1024L * 1024L * 1024L * 1024L); - case "GB" -> (long) (value * 1024L * 1024L * 1024L); - case "MB" -> (long) (value * 1024L * 1024L); - case "KB" -> (long) (value * 1024L); - case "B" -> (long) value; - default -> (long) (value * 1024L * 1024L); // Default to MB + case "TB" -> toBytes(value, 4); + case "GB" -> toBytes(value, 3); + case "MB" -> toBytes(value, 2); + case "KB" -> toBytes(value, 1); + case "B" -> toBytes(value, 0); + default -> toBytes(value, 2); // Default to MB }; } } catch (NumberFormatException e) { @@ -590,6 +583,30 @@ public class GeneralUtils { return convertSizeToBytes(sizeStr, "MB"); } + private Long toBytes(BigDecimal value, int powerOf1024) { + if (value == null) { + return null; + } + if (value.compareTo(BigDecimal.ZERO) < 0) { + log.warn("Size value cannot be negative: {}", value); + return null; + } + if (powerOf1024 < 0 || powerOf1024 > 4) { + throw new IllegalArgumentException("Invalid power for size conversion: " + powerOf1024); + } + BigDecimal multiplier = powerOf1024 == 0 ? BigDecimal.ONE : KIB.pow(powerOf1024); + BigDecimal bytes = value.multiply(multiplier).setScale(0, RoundingMode.DOWN); + if (bytes.compareTo(LONG_MAX_DECIMAL) > 0) { + log.warn("Size value too large to fit in long: {}", bytes); + return null; + } + return bytes.longValue(); + } + + private BigDecimal parseSizeValue(String value) { + return new BigDecimal(value); + } + /* Validates if a string represents a valid size unit. */ private boolean isValidSizeUnit(String unit) { // Use a precomputed Set for O(1) lookup, normalize using a locale-safe toUpperCase diff --git a/app/common/src/test/java/stirling/software/common/util/GeneralUtilsAdditionalTest.java b/app/common/src/test/java/stirling/software/common/util/GeneralUtilsAdditionalTest.java index ac913353b..27a4bc7c2 100644 --- a/app/common/src/test/java/stirling/software/common/util/GeneralUtilsAdditionalTest.java +++ b/app/common/src/test/java/stirling/software/common/util/GeneralUtilsAdditionalTest.java @@ -16,6 +16,14 @@ class GeneralUtilsAdditionalTest { assertNull(GeneralUtils.convertSizeToBytes(null)); } + @Test + void testConvertSizeToBytesEdgeCases() { + assertNull(GeneralUtils.convertSizeToBytes("-10MB")); + assertNull(GeneralUtils.convertSizeToBytes("10000000TB")); // overflow beyond long + assertEquals(1099511627776L, GeneralUtils.convertSizeToBytes("1TB")); + assertEquals(2684354560L, GeneralUtils.convertSizeToBytes("2.5GB")); + } + @Test void testFormatBytes() { assertEquals("512 B", GeneralUtils.formatBytes(512));