From b71031c6f7725cb4dc080a3d072ecb6264e29d46 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Wed, 5 Feb 2025 16:38:46 +0100 Subject: [PATCH 1/2] improve BlocksByRangeMessage validation --- .../methods/BeaconBlocksByRangeMessageHandler.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandler.java b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandler.java index b84454074d7..0faf265fa9e 100644 --- a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandler.java +++ b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandler.java @@ -95,10 +95,18 @@ public Optional validateRequest( return Optional.of(new RpcException(INVALID_REQUEST_CODE, "Step must be greater than zero")); } - final UInt64 maxRequestBlocks = - spec.forMilestone(latestMilestoneRequested).miscHelpers().getMaxRequestBlocks(); + final int maxRequestBlocks = + spec.forMilestone(latestMilestoneRequested).miscHelpers().getMaxRequestBlocks().intValue(); + + int requestedCount; + try { + requestedCount = request.getCount().intValue(); + } catch (final ArithmeticException __) { + // handle overflows + requestedCount = -1; + } - if (request.getCount().isGreaterThan(maxRequestBlocks)) { + if (requestedCount == -1 || requestedCount > maxRequestBlocks) { requestCounter.labels("count_too_big").inc(); return Optional.of( new RpcException( From 175cf6ab7acb0db628ddcd4b9820fa0747e1199b Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Wed, 5 Feb 2025 17:15:54 +0100 Subject: [PATCH 2/2] additional check and tests --- .../BeaconBlocksByRangeMessageHandler.java | 11 ++++-- ...BeaconBlocksByRangeMessageHandlerTest.java | 35 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandler.java b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandler.java index 0faf265fa9e..8bed124db0f 100644 --- a/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandler.java +++ b/networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandler.java @@ -80,8 +80,15 @@ public BeaconBlocksByRangeMessageHandler( public Optional validateRequest( final String protocolId, final BeaconBlocksByRangeRequestMessage request) { final int version = BeaconChainMethodIds.extractBeaconBlocksByRangeVersion(protocolId); - final SpecMilestone latestMilestoneRequested = - spec.getForkSchedule().getSpecMilestoneAtSlot(request.getMaxSlot()); + final SpecMilestone latestMilestoneRequested; + try { + latestMilestoneRequested = + spec.getForkSchedule().getSpecMilestoneAtSlot(request.getMaxSlot()); + } catch (final ArithmeticException __) { + return Optional.of( + new RpcException(INVALID_REQUEST_CODE, "Requested slot is too far in the future")); + } + final boolean isAltairActive = latestMilestoneRequested.isGreaterThanOrEqualTo(SpecMilestone.ALTAIR); diff --git a/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandlerTest.java b/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandlerTest.java index 0c2bdfad003..ba2902e0493 100644 --- a/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandlerTest.java +++ b/networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/rpc/beaconchain/methods/BeaconBlocksByRangeMessageHandlerTest.java @@ -192,6 +192,41 @@ public void validateRequest_shouldRejectRequestWhenCountIsTooBig() { "Only a maximum of 1024 blocks can be requested per request")); } + @Test + public void validateRequest_shouldRejectRequestWhenCountIsTooBigDueToIntOverflow() { + final int startBlock = 15; + final int skip = 1; + + final Optional result = + handler.validateRequest( + protocolId, + new BeaconBlocksByRangeRequestMessage( + UInt64.valueOf(startBlock), + UInt64.valueOf(((long) Integer.MAX_VALUE) + 1), + UInt64.valueOf(skip))); + + assertThat(result) + .hasValue( + new RpcException( + INVALID_REQUEST_CODE, + "Only a maximum of 1024 blocks can be requested per request")); + } + + @Test + public void validateRequest_shouldRejectRequestWhenGetMaxSlotOverflows() { + final int skip = 1; + + final Optional result = + handler.validateRequest( + protocolId, + new BeaconBlocksByRangeRequestMessage( + UInt64.MAX_VALUE, UInt64.valueOf(10), UInt64.valueOf(skip))); + + assertThat(result) + .hasValue( + new RpcException(INVALID_REQUEST_CODE, "Requested slot is too far in the future")); + } + @Test public void validateRequest_shouldRejectRequestWhenCountIsTooBigForDeneb() { final int startBlock = 15;