mirror of
https://github.com/arsvendg/Stirling-PDF.git
synced 2026-07-16 19:33:11 +02:00
fix(common): 🛡️CWE-681 & CWE-197 eliminate tainted numeric casts in size parsing by using BigDecimal with range guards (#5521)
# 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.
This commit is contained in:
@@ -4,6 +4,8 @@ import java.io.File;
|
|||||||
import java.io.FileOutputStream;
|
import java.io.FileOutputStream;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.io.InputStream;
|
import java.io.InputStream;
|
||||||
|
import java.math.BigDecimal;
|
||||||
|
import java.math.RoundingMode;
|
||||||
import java.net.*;
|
import java.net.*;
|
||||||
import java.nio.charset.StandardCharsets;
|
import java.nio.charset.StandardCharsets;
|
||||||
import java.nio.file.*;
|
import java.nio.file.*;
|
||||||
@@ -38,6 +40,10 @@ public class GeneralUtils {
|
|||||||
*/
|
*/
|
||||||
private static final int MAX_DNS_ADDRESSES = 20;
|
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<String> DEFAULT_VALID_SCRIPTS = Set.of("png_to_webp.py", "split_photos.py");
|
private final Set<String> DEFAULT_VALID_SCRIPTS = Set.of("png_to_webp.py", "split_photos.py");
|
||||||
private final Set<String> DEFAULT_VALID_PIPELINE =
|
private final Set<String> DEFAULT_VALID_PIPELINE =
|
||||||
Set.of(
|
Set.of(
|
||||||
@@ -539,39 +545,26 @@ public class GeneralUtils {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
if (sizeStr.endsWith("TB")) {
|
if (sizeStr.endsWith("TB")) {
|
||||||
return (long)
|
return toBytes(parseSizeValue(sizeStr.substring(0, sizeStr.length() - 2)), 4);
|
||||||
(Double.parseDouble(sizeStr.substring(0, sizeStr.length() - 2))
|
|
||||||
* 1024L
|
|
||||||
* 1024L
|
|
||||||
* 1024L
|
|
||||||
* 1024L);
|
|
||||||
} else if (sizeStr.endsWith("GB")) {
|
} else if (sizeStr.endsWith("GB")) {
|
||||||
return (long)
|
return toBytes(parseSizeValue(sizeStr.substring(0, sizeStr.length() - 2)), 3);
|
||||||
(Double.parseDouble(sizeStr.substring(0, sizeStr.length() - 2))
|
|
||||||
* 1024L
|
|
||||||
* 1024L
|
|
||||||
* 1024L);
|
|
||||||
} else if (sizeStr.endsWith("MB")) {
|
} else if (sizeStr.endsWith("MB")) {
|
||||||
return (long)
|
return toBytes(parseSizeValue(sizeStr.substring(0, sizeStr.length() - 2)), 2);
|
||||||
(Double.parseDouble(sizeStr.substring(0, sizeStr.length() - 2))
|
|
||||||
* 1024L
|
|
||||||
* 1024L);
|
|
||||||
} else if (sizeStr.endsWith("KB")) {
|
} else if (sizeStr.endsWith("KB")) {
|
||||||
return (long)
|
return toBytes(parseSizeValue(sizeStr.substring(0, sizeStr.length() - 2)), 1);
|
||||||
(Double.parseDouble(sizeStr.substring(0, sizeStr.length() - 2)) * 1024L);
|
|
||||||
} else if (!sizeStr.isEmpty() && sizeStr.charAt(sizeStr.length() - 1) == 'B') {
|
} 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 {
|
} else {
|
||||||
// Use provided default unit or fall back to MB
|
// Use provided default unit or fall back to MB
|
||||||
String unit = defaultUnit != null ? defaultUnit.toUpperCase(Locale.ROOT) : "MB";
|
String unit = defaultUnit != null ? defaultUnit.toUpperCase(Locale.ROOT) : "MB";
|
||||||
double value = Double.parseDouble(sizeStr);
|
BigDecimal value = parseSizeValue(sizeStr);
|
||||||
return switch (unit) {
|
return switch (unit) {
|
||||||
case "TB" -> (long) (value * 1024L * 1024L * 1024L * 1024L);
|
case "TB" -> toBytes(value, 4);
|
||||||
case "GB" -> (long) (value * 1024L * 1024L * 1024L);
|
case "GB" -> toBytes(value, 3);
|
||||||
case "MB" -> (long) (value * 1024L * 1024L);
|
case "MB" -> toBytes(value, 2);
|
||||||
case "KB" -> (long) (value * 1024L);
|
case "KB" -> toBytes(value, 1);
|
||||||
case "B" -> (long) value;
|
case "B" -> toBytes(value, 0);
|
||||||
default -> (long) (value * 1024L * 1024L); // Default to MB
|
default -> toBytes(value, 2); // Default to MB
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
} catch (NumberFormatException e) {
|
} catch (NumberFormatException e) {
|
||||||
@@ -590,6 +583,30 @@ public class GeneralUtils {
|
|||||||
return convertSizeToBytes(sizeStr, "MB");
|
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. */
|
/* Validates if a string represents a valid size unit. */
|
||||||
private boolean isValidSizeUnit(String unit) {
|
private boolean isValidSizeUnit(String unit) {
|
||||||
// Use a precomputed Set for O(1) lookup, normalize using a locale-safe toUpperCase
|
// Use a precomputed Set for O(1) lookup, normalize using a locale-safe toUpperCase
|
||||||
|
|||||||
@@ -16,6 +16,14 @@ class GeneralUtilsAdditionalTest {
|
|||||||
assertNull(GeneralUtils.convertSizeToBytes(null));
|
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
|
@Test
|
||||||
void testFormatBytes() {
|
void testFormatBytes() {
|
||||||
assertEquals("512 B", GeneralUtils.formatBytes(512));
|
assertEquals("512 B", GeneralUtils.formatBytes(512));
|
||||||
|
|||||||
Reference in New Issue
Block a user