From 568beb6a30e2ebc88100c46d0b50b874dcc1ac2a Mon Sep 17 00:00:00 2001 From: thc202 Date: Wed, 2 Oct 2024 16:43:58 +0100 Subject: [PATCH] fuzz: address lossy conversion warns Sum the number of payloads taking into account the possible overflows. Signed-off-by: thc202 --- addOns/fuzz/CHANGELOG.md | 1 + .../fuzz/FuzzerPayloadGeneratorUIHandler.java | 12 +-- .../fuzz/impl/FuzzLocationTableEntry.java | 10 ++- .../zap/extension/fuzz/impl/Utils.java | 47 +++++++++++ .../generator/CompositePayloadGenerator.java | 8 +- .../extension/fuzz/impl/UtilsUnitTest.java | 81 +++++++++++++++++++ 6 files changed, 145 insertions(+), 14 deletions(-) create mode 100644 addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/impl/Utils.java create mode 100644 addOns/fuzz/src/test/java/org/zaproxy/zap/extension/fuzz/impl/UtilsUnitTest.java diff --git a/addOns/fuzz/CHANGELOG.md b/addOns/fuzz/CHANGELOG.md index 70056d0b0d5..83abc29c480 100644 --- a/addOns/fuzz/CHANGELOG.md +++ b/addOns/fuzz/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased ### Changed +- Maintenance changes. - Replace library used for regex payload generation, to address performance and compatibility issues. ## [13.13.0] - 2024-05-07 diff --git a/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/FuzzerPayloadGeneratorUIHandler.java b/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/FuzzerPayloadGeneratorUIHandler.java index 1f1fe4cde58..34d78433783 100644 --- a/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/FuzzerPayloadGeneratorUIHandler.java +++ b/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/FuzzerPayloadGeneratorUIHandler.java @@ -56,6 +56,7 @@ import org.parosproxy.paros.Constant; import org.zaproxy.zap.extension.fuzz.ExtensionFuzz.FuzzersDirChangeListener; import org.zaproxy.zap.extension.fuzz.FuzzerPayloadGeneratorUIHandler.FuzzerPayloadGeneratorUI; +import org.zaproxy.zap.extension.fuzz.impl.Utils; import org.zaproxy.zap.extension.fuzz.payloads.DefaultPayload; import org.zaproxy.zap.extension.fuzz.payloads.generator.FileStringPayloadGenerator; import org.zaproxy.zap.extension.fuzz.payloads.generator.PayloadGenerator; @@ -105,7 +106,7 @@ public static class FuzzerPayloadGeneratorUI implements PayloadGeneratorUI { private final List selectedFuzzers; - private int numberOfPayloads; + private long numberOfPayloads; private Path file; private String description; @@ -173,10 +174,11 @@ public String getDescription() { @Override public long getNumberOfPayloads() { if (numberOfPayloads == -1) { - numberOfPayloads = 0; - for (FuzzerPayloadSource selectedFuzzer : selectedFuzzers) { - numberOfPayloads += selectedFuzzer.getPayloadGenerator().getNumberOfPayloads(); - } + numberOfPayloads = + Utils.sum( + selectedFuzzers.stream() + .map(FuzzerPayloadSource::getPayloadGenerator) + .mapToLong(PayloadGenerator::getNumberOfPayloads)); } return numberOfPayloads; } diff --git a/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/impl/FuzzLocationTableEntry.java b/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/impl/FuzzLocationTableEntry.java index f5e7a9140f8..436d082832b 100644 --- a/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/impl/FuzzLocationTableEntry.java +++ b/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/impl/FuzzLocationTableEntry.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.zaproxy.zap.extension.fuzz.payloads.ui.PayloadGeneratorUI; import org.zaproxy.zap.model.MessageLocation; import org.zaproxy.zap.view.messagelocation.MessageLocationHighlight; import org.zaproxy.zap.view.messagelocation.MessageLocationTableEntry; @@ -53,10 +54,11 @@ public FuzzLocationTableEntry( public void setPayloads(List payloads) { this.payloads = new ArrayList<>(payloads); - numberOfPayloads = 0; - for (PayloadTableEntry payloadTableEntry : payloads) { - numberOfPayloads += payloadTableEntry.getPayloadGeneratorUI().getNumberOfPayloads(); - } + numberOfPayloads = + Utils.sumLongToInt( + payloads.stream() + .map(PayloadTableEntry::getPayloadGeneratorUI) + .mapToLong(PayloadGeneratorUI::getNumberOfPayloads)); } public List getPayloads() { diff --git a/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/impl/Utils.java b/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/impl/Utils.java new file mode 100644 index 00000000000..0cad7f86c8b --- /dev/null +++ b/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/impl/Utils.java @@ -0,0 +1,47 @@ +/* + * Zed Attack Proxy (ZAP) and its related class files. + * + * ZAP is an HTTP/HTTPS proxy for assessing web application security. + * + * Copyright 2024 The ZAP Development Team + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.zaproxy.zap.extension.fuzz.impl; + +import java.util.stream.LongStream; + +public final class Utils { + + private Utils() {} + + public static int sumLongToInt(LongStream values) { + try { + long result = values.reduce(0, Math::addExact); + if (result > Integer.MAX_VALUE) { + return Integer.MAX_VALUE; + } + return (int) result; + } catch (ArithmeticException e) { + return Integer.MAX_VALUE; + } + } + + public static long sum(LongStream values) { + try { + return values.reduce(0, Math::addExact); + } catch (ArithmeticException e) { + return Long.MAX_VALUE; + } + } +} diff --git a/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/payloads/generator/CompositePayloadGenerator.java b/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/payloads/generator/CompositePayloadGenerator.java index bac10dde99e..a53e98da35b 100644 --- a/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/payloads/generator/CompositePayloadGenerator.java +++ b/addOns/fuzz/src/main/java/org/zaproxy/zap/extension/fuzz/payloads/generator/CompositePayloadGenerator.java @@ -23,6 +23,7 @@ import java.util.Iterator; import java.util.List; import org.apache.commons.collections.iterators.IteratorChain; +import org.zaproxy.zap.extension.fuzz.impl.Utils; import org.zaproxy.zap.extension.fuzz.payloads.Payload; import org.zaproxy.zap.utils.EmptyResettableAutoCloseableIterator; import org.zaproxy.zap.utils.ResettableAutoCloseableIterator; @@ -48,11 +49,8 @@ public CompositePayloadGenerator(List> payloadGenerators) { @Override public long getNumberOfPayloads() { - int size = 0; - for (PayloadGenerator payloadGenerator : payloadGenerators) { - size += payloadGenerator.getNumberOfPayloads(); - } - return size; + return Utils.sum( + payloadGenerators.stream().mapToLong(PayloadGenerator::getNumberOfPayloads)); } @Override diff --git a/addOns/fuzz/src/test/java/org/zaproxy/zap/extension/fuzz/impl/UtilsUnitTest.java b/addOns/fuzz/src/test/java/org/zaproxy/zap/extension/fuzz/impl/UtilsUnitTest.java new file mode 100644 index 00000000000..86cb18b1b99 --- /dev/null +++ b/addOns/fuzz/src/test/java/org/zaproxy/zap/extension/fuzz/impl/UtilsUnitTest.java @@ -0,0 +1,81 @@ +/* + * Zed Attack Proxy (ZAP) and its related class files. + * + * ZAP is an HTTP/HTTPS proxy for assessing web application security. + * + * Copyright 2024 The ZAP Development Team + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.zaproxy.zap.extension.fuzz.impl; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +import java.util.stream.LongStream; +import org.junit.jupiter.api.Test; + +/** Unit test for {@link Utils}. */ +class UtilsUnitTest { + + @Test + void shouldSumLongs() { + // Given + var values = LongStream.of(1L, 2L); + // When + long result = Utils.sum(values); + // Then + assertThat(result, is(equalTo(3L))); + } + + @Test + void shouldSumLongsHandlingOverflow() { + // Given + var values = LongStream.of(Long.MAX_VALUE, Long.MAX_VALUE); + // When + long result = Utils.sum(values); + // Then + assertThat(result, is(equalTo(Long.MAX_VALUE))); + } + + @Test + void shouldSumLongsToInt() { + // Given + var values = LongStream.of(1L, 2L); + // When + int result = Utils.sumLongToInt(values); + // Then + assertThat(result, is(equalTo(3))); + } + + @Test + void shouldSumLongsToIntHandlingLongOverflow() { + // Given + var values = LongStream.of(Long.MAX_VALUE, Long.MAX_VALUE); + // When + int result = Utils.sumLongToInt(values); + // Then + assertThat(result, is(equalTo(Integer.MAX_VALUE))); + } + + @Test + void shouldSumLongsToIntHandlingIntegerOverflow() { + // Given + var values = LongStream.of(Integer.MAX_VALUE, 1L); + // When + int result = Utils.sumLongToInt(values); + // Then + assertThat(result, is(equalTo(Integer.MAX_VALUE))); + } +}