From 658751e57732044d09f41332aef93e8b399b8fa6 Mon Sep 17 00:00:00 2001 From: Kim Rader Date: Tue, 17 Dec 2024 15:55:20 -0800 Subject: [PATCH] Fix NPE when senderId or receiverId is null in a tokenClaimAirdrop transaction Signed-off-by: Kim Rader --- .../handlers/TokenClaimAirdropHandler.java | 6 ++++ .../TokenClaimAirdropHandlerTest.java | 28 +++++++++++++++++++ .../utilops/mod/BodyIdClearingStrategy.java | 8 +++++- .../suites/hip904/TokenClaimAirdropTest.java | 19 +++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/TokenClaimAirdropHandler.java b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/TokenClaimAirdropHandler.java index 5d0b1bc95a9a..59059b4233d1 100644 --- a/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/TokenClaimAirdropHandler.java +++ b/hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/TokenClaimAirdropHandler.java @@ -18,6 +18,7 @@ import static com.hedera.hapi.node.base.ResponseCodeEnum.EMPTY_PENDING_AIRDROP_ID_LIST; import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_ACCOUNT_ID; +import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_PENDING_AIRDROP_ID; import static com.hedera.hapi.node.base.ResponseCodeEnum.PENDING_AIRDROP_ID_LIST_TOO_LONG; import static com.hedera.hapi.node.base.ResponseCodeEnum.PENDING_AIRDROP_ID_REPEATED; import static com.hedera.hapi.node.base.ResponseCodeEnum.TOKEN_AIRDROP_WITH_FALLBACK_ROYALTY; @@ -114,6 +115,11 @@ public void pureChecks(@NonNull TransactionBody txn) throws PreCheckException { final var uniqueAirdrops = Set.copyOf(pendingAirdrops); validateTruePreCheck(pendingAirdrops.size() == uniqueAirdrops.size(), PENDING_AIRDROP_ID_REPEATED); + + validateTruePreCheck( + pendingAirdrops.stream().allMatch(PendingAirdropId::hasSenderId), INVALID_PENDING_AIRDROP_ID); + validateTruePreCheck( + pendingAirdrops.stream().allMatch(PendingAirdropId::hasReceiverId), INVALID_PENDING_AIRDROP_ID); } @Override diff --git a/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/TokenClaimAirdropHandlerTest.java b/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/TokenClaimAirdropHandlerTest.java index 20ef4eff705e..7c7a06999642 100644 --- a/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/TokenClaimAirdropHandlerTest.java +++ b/hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/TokenClaimAirdropHandlerTest.java @@ -169,6 +169,34 @@ void pureChecksHasValidPath() { .doesNotThrowAnyException(); } + @Test + void pureChecksEmptySenderThrows() { + final List pendingAirdropIds = new ArrayList<>(); + pendingAirdropIds.add(PendingAirdropId.newBuilder() + .receiverId(ACCOUNT_ID_3333) + .fungibleTokenType(TOKEN_2468) + .build()); + final var txn = newTokenClaimAirdrop(TokenClaimAirdropTransactionBody.newBuilder() + .pendingAirdrops(pendingAirdropIds) + .build()); + Assertions.assertThatThrownBy(() -> tokenClaimAirdropHandler.pureChecks(txn)) + .isInstanceOf(PreCheckException.class); + } + + @Test + void pureChecksEmptyReceiverThrows() { + final List pendingAirdropIds = new ArrayList<>(); + pendingAirdropIds.add(PendingAirdropId.newBuilder() + .senderId(ACCOUNT_ID_4444) + .fungibleTokenType(TOKEN_2468) + .build()); + final var txn = newTokenClaimAirdrop(TokenClaimAirdropTransactionBody.newBuilder() + .pendingAirdrops(pendingAirdropIds) + .build()); + Assertions.assertThatThrownBy(() -> tokenClaimAirdropHandler.pureChecks(txn)) + .isInstanceOf(PreCheckException.class); + } + @Test void preHandleAccountNotExistPath() throws PreCheckException { final List pendingAirdropIds = new ArrayList<>(); diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/utilops/mod/BodyIdClearingStrategy.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/utilops/mod/BodyIdClearingStrategy.java index 86dbf7ea35af..3a6d1a6cac38 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/utilops/mod/BodyIdClearingStrategy.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/utilops/mod/BodyIdClearingStrategy.java @@ -25,6 +25,7 @@ import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_ETHEREUM_TRANSACTION; import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_FILE_ID; import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_NODE_ACCOUNT; +import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_PENDING_AIRDROP_ID; import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_SCHEDULE_ID; import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_SIGNATURE; import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.INVALID_TOKEN_ID; @@ -141,7 +142,12 @@ public class BodyIdClearingStrategy extends IdClearingStrategy entry( "proto.EthereumTransactionBody.call_data", ExpectedResponse.atConsensus(INVALID_ETHEREUM_TRANSACTION)), - entry("proto.TokenUpdateNftsTransactionBody.token", ExpectedResponse.atIngest(INVALID_TOKEN_ID))); + entry("proto.TokenUpdateNftsTransactionBody.token", ExpectedResponse.atIngest(INVALID_TOKEN_ID)), + entry("proto.PendingAirdropId.receiver_id", ExpectedResponse.atIngest(INVALID_PENDING_AIRDROP_ID)), + entry( + "proto.PendingAirdropId.fungible_token_type", + ExpectedResponse.atConsensus(INVALID_PENDING_AIRDROP_ID)), + entry("proto.PendingAirdropId.sender_id", ExpectedResponse.atIngest(INVALID_PENDING_AIRDROP_ID))); private static final Map SCHEDULED_CLEARED_ID_RESPONSES = Map.ofEntries( entry("proto.AccountAmount.accountID", ExpectedResponse.atConsensusOneOf(INVALID_ACCOUNT_ID))); diff --git a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/hip904/TokenClaimAirdropTest.java b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/hip904/TokenClaimAirdropTest.java index 1c2436aecbc5..3cef7685f4f6 100644 --- a/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/hip904/TokenClaimAirdropTest.java +++ b/hedera-node/test-clients/src/main/java/com/hedera/services/bdd/suites/hip904/TokenClaimAirdropTest.java @@ -57,8 +57,10 @@ import static com.hedera.services.bdd.spec.utilops.UtilVerbs.logIt; import static com.hedera.services.bdd.spec.utilops.UtilVerbs.newKeyNamed; import static com.hedera.services.bdd.spec.utilops.UtilVerbs.overriding; +import static com.hedera.services.bdd.spec.utilops.UtilVerbs.submitModified; import static com.hedera.services.bdd.spec.utilops.UtilVerbs.validateChargedUsd; import static com.hedera.services.bdd.spec.utilops.UtilVerbs.withOpContext; +import static com.hedera.services.bdd.spec.utilops.mod.ModificationUtils.withSuccessivelyVariedBodyIds; import static com.hedera.services.bdd.suites.HapiSuite.DEFAULT_PAYER; import static com.hedera.services.bdd.suites.HapiSuite.ONE_HBAR; import static com.hedera.services.bdd.suites.HapiSuite.ONE_HUNDRED_HBARS; @@ -222,6 +224,23 @@ final Stream claimFungibleTokenAirdrop() { getAccountInfo(RECEIVER).hasToken(relationshipWith(NON_FUNGIBLE_TOKEN))); } + @HapiTest + @DisplayName("fails gracefully with null parameters") + final Stream idVariantsTreatedAsExpected() { + return hapiTest( + cryptoCreate(OWNER).balance(ONE_HUNDRED_HBARS), + cryptoCreate(RECEIVER_WITH_0_AUTO_ASSOCIATIONS) + .balance(ONE_HUNDRED_HBARS) + .maxAutomaticTokenAssociations(0), + createFT(FUNGIBLE_TOKEN_1, OWNER, 1000L), + tokenAirdrop(moving(1, FUNGIBLE_TOKEN_1).between(OWNER, RECEIVER_WITH_0_AUTO_ASSOCIATIONS)) + .payingWith(OWNER), + submitModified(withSuccessivelyVariedBodyIds(), () -> tokenClaimAirdrop( + pendingAirdrop(OWNER, RECEIVER_WITH_0_AUTO_ASSOCIATIONS, FUNGIBLE_TOKEN_1)) + .signedBy(DEFAULT_PAYER, RECEIVER_WITH_0_AUTO_ASSOCIATIONS) + .payingWith(RECEIVER_WITH_0_AUTO_ASSOCIATIONS))); + } + @HapiTest @DisplayName("single token claim success that receiver paying for it") final Stream singleTokenClaimSuccessThatReceiverPayingForIt() {