Skip to content

Commit

Permalink
fix: Validate CustomFees input arrays in `UpdateTokenCustomFeesDeco…
Browse files Browse the repository at this point in the history 
…der` (#15520)

Signed-off-by: Stanimir Stoyanov <[email protected]>
  • Loading branch information
@stoyanov-st
stoyanov-st committed Sep 18, 2024
1 parent 0fea101 commit 6c987c2
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 5 deletions.
18 changes: 18 additions & 0 deletions ...act/impl/exec/systemcontracts/hts/updatetokencustomfees/UpdateTokenCustomFeesDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@

package com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.updatetokencustomfees;

import static com.hedera.node.app.spi.workflows.HandleException.validateFalse;

import com.esaulpaugh.headlong.abi.Address;
import com.esaulpaugh.headlong.abi.Tuple;
import com.hedera.hapi.node.base.Fraction;
import com.hedera.hapi.node.base.ResponseCodeEnum;
import com.hedera.hapi.node.base.TokenID;
import com.hedera.hapi.node.token.TokenFeeScheduleUpdateTransactionBody;
import com.hedera.hapi.node.transaction.CustomFee;
Expand All @@ -29,6 +32,7 @@
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.AddressIdConverter;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.HtsCallAttempt;
import com.hedera.node.app.service.contract.impl.utils.ConversionUtils;
import com.hedera.node.config.data.TokensConfig;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import java.util.Arrays;
Expand Down Expand Up @@ -93,6 +97,7 @@ public TransactionBody decodeUpdateFungibleTokenCustomFees(@NonNull final HtsCal
throws IllegalArgumentException {
final var call = UpdateTokenCustomFeesTranslator.UPDATE_FUNGIBLE_TOKEN_CUSTOM_FEES_FUNCTION.decodeCall(
attempt.inputBytes());
validateCustomFeesLength(attempt, call);
return TransactionBody.newBuilder()
.tokenFeeScheduleUpdate(
updateTokenCustomFees(call, attempt.addressIdConverter(), this::customFungibleFees))
Expand All @@ -110,6 +115,7 @@ public TransactionBody decodeUpdateNonFungibleTokenCustomFees(@NonNull final Hts
throws IllegalArgumentException {
final var call = UpdateTokenCustomFeesTranslator.UPDATE_NON_FUNGIBLE_TOKEN_CUSTOM_FEES_FUNCTION.decodeCall(
attempt.inputBytes());
validateCustomFeesLength(attempt, call);
return TransactionBody.newBuilder()
.tokenFeeScheduleUpdate(
updateTokenCustomFees(call, attempt.addressIdConverter(), this::customNonFungibleFees))
Expand Down Expand Up @@ -219,4 +225,16 @@ private TokenID determineDenominatingToken(@NonNull final Tuple fee, @NonNull fi
tokenAddress, fee.get(ROYALTY_FALLBACK_FEE_USE_HBARS_FOR_PAYMENT)))
.build();
}

private void validateCustomFeesLength(@NonNull final HtsCallAttempt attempt, @NonNull final Tuple call) {
final var maxAllowedCustomFees =
attempt.configuration().getConfigData(TokensConfig.class).maxCustomFeesAllowed();
final var fixedFee = (Tuple[]) call.get(FIXED_FEE);
final var fractionalOrRoyaltyFee = (Tuple[]) call.get(ROYALTY_FEE);
validateFalse(fixedFee.length > maxAllowedCustomFees, ResponseCodeEnum.CUSTOM_FEES_LIST_TOO_LONG);
validateFalse(fractionalOrRoyaltyFee.length > maxAllowedCustomFees, ResponseCodeEnum.CUSTOM_FEES_LIST_TOO_LONG);
validateFalse(
fixedFee.length + fractionalOrRoyaltyFee.length > maxAllowedCustomFees,
ResponseCodeEnum.CUSTOM_FEES_LIST_TOO_LONG);
}
}
120 changes: 119 additions & 1 deletion ...test/exec/systemcontracts/hts/updatetokencustomfees/UpdateTokenCustomFeesDecoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.OWNER_HEADLONG_ADDRESS;
import static com.hedera.node.app.service.contract.impl.test.TestHelpers.OWNER_ID;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.lenient;

import com.esaulpaugh.headlong.abi.Address;
import com.esaulpaugh.headlong.abi.Tuple;
import com.hedera.hapi.node.base.Fraction;
import com.hedera.hapi.node.base.ResponseCodeEnum;
import com.hedera.hapi.node.transaction.CustomFee;
import com.hedera.hapi.node.transaction.FixedFee;
import com.hedera.hapi.node.transaction.FractionalFee;
Expand All @@ -37,6 +40,9 @@
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.HtsCallAttempt;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.updatetokencustomfees.UpdateTokenCustomFeesDecoder;
import com.hedera.node.app.service.contract.impl.exec.systemcontracts.hts.updatetokencustomfees.UpdateTokenCustomFeesTranslator;
import com.hedera.node.app.spi.workflows.HandleException;
import com.hedera.node.config.data.TokensConfig;
import com.swirlds.config.api.Configuration;
import java.util.List;
import org.apache.tuweni.bytes.Bytes;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -53,6 +59,12 @@ class UpdateTokenCustomFeesDecoderTest {
@Mock
private AddressIdConverter addressIdConverter;

@Mock
Configuration configuration;

@Mock
private TokensConfig tokensConfig;

private UpdateTokenCustomFeesDecoder subject;

private final Address zeroAddress = Address.wrap("0x0000000000000000000000000000000000000000");
Expand Down Expand Up @@ -122,7 +134,7 @@ class UpdateTokenCustomFeesDecoderTest {
@BeforeEach
void setUp() {
subject = new UpdateTokenCustomFeesDecoder();
given(attempt.addressIdConverter()).willReturn(addressIdConverter);
lenient().when(attempt.addressIdConverter()).thenReturn(addressIdConverter);
}

@Test
Expand All @@ -136,6 +148,7 @@ void decodeValidUpdateFungibleTokenCustomHBarFeesRequest() {
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateFungibleTokenCustomFees(attempt);
Expand All @@ -158,6 +171,7 @@ void decodeValidUpdateFungibleTokenCustomHBarAndHTSFeesRequest() {
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateFungibleTokenCustomFees(attempt);
Expand Down Expand Up @@ -185,6 +199,7 @@ void decodeValidUpdateFungibleTokenCustomFixedFeesRequest() {
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateFungibleTokenCustomFees(attempt);
Expand Down Expand Up @@ -219,6 +234,7 @@ void decodeValidUpdateFungibleTokenCustom10FixedFeesRequest() {
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateFungibleTokenCustomFees(attempt);
Expand All @@ -230,6 +246,35 @@ void decodeValidUpdateFungibleTokenCustom10FixedFeesRequest() {
assertTrue(updateTokenCustomFees.customFees().contains(FIXED_HBAR_FEES));
}

@Test
void decodeValidUpdateFungibleTokenCustom11FixedFeesRequest() {
// Given a valid encoded update token custom fees request
final var encoded = Bytes.wrapByteBuffer(
UpdateTokenCustomFeesTranslator.UPDATE_FUNGIBLE_TOKEN_CUSTOM_FEES_FUNCTION.encodeCallWithArgs(
FUNGIBLE_TOKEN_HEADLONG_ADDRESS,
new Tuple[] {
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE
},
EMPTY_TOKEN_FEES_TUPLE_ARR))
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
setConfiguration();

final var error =
assertThrows(HandleException.class, () -> subject.decodeUpdateFungibleTokenCustomFees(attempt));
assertEquals(ResponseCodeEnum.CUSTOM_FEES_LIST_TOO_LONG, error.getStatus());
}

@Test
void decodeValidUpdateFungibleTokenCustomHTSFeesRequest() {
// Given a valid encoded update token custom fees request
Expand All @@ -241,6 +286,7 @@ void decodeValidUpdateFungibleTokenCustomHTSFeesRequest() {
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateFungibleTokenCustomFees(attempt);
Expand All @@ -263,6 +309,7 @@ void decodeValidUpdateFungibleTokenCustomHTSSameTokenForPaymentFeesRequest() {
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateFungibleTokenCustomFees(attempt);
Expand All @@ -285,6 +332,7 @@ void decodeValidUpdateFungibleTokenCustomFractionalFeesRequest() {
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateFungibleTokenCustomFees(attempt);
Expand All @@ -308,6 +356,7 @@ void decodeValidUpdateNonFungibleTokenCustomRoyaltyFeesRequest() {
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateNonFungibleTokenCustomFees(attempt);
Expand All @@ -319,6 +368,65 @@ void decodeValidUpdateNonFungibleTokenCustomRoyaltyFeesRequest() {
assertEquals(updateTokenCustomFees.customFees().get(0), ROYALTY_FEE);
}

@Test
void decodeValidUpdateNonFungibleTokenCustom11RoyaltyFeesRequest() {
// Given a valid encoded update token custom fees request
final var encoded = Bytes.wrapByteBuffer(
UpdateTokenCustomFeesTranslator.UPDATE_NON_FUNGIBLE_TOKEN_CUSTOM_FEES_FUNCTION
.encodeCallWithArgs(
NON_FUNGIBLE_TOKEN_HEADLONG_ADDRESS, EMPTY_TOKEN_FEES_TUPLE_ARR, new Tuple[] {
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE
}))
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
setConfiguration();

final var error =
assertThrows(HandleException.class, () -> subject.decodeUpdateNonFungibleTokenCustomFees(attempt));
assertEquals(ResponseCodeEnum.CUSTOM_FEES_LIST_TOO_LONG, error.getStatus());
}

@Test
void decodeValidUpdateNonFungibleTokenCustom11MixedFeesRequest() {
// Given a valid encoded update token custom fees request
final var encoded = Bytes.wrapByteBuffer(
UpdateTokenCustomFeesTranslator.UPDATE_NON_FUNGIBLE_TOKEN_CUSTOM_FEES_FUNCTION
.encodeCallWithArgs(
NON_FUNGIBLE_TOKEN_HEADLONG_ADDRESS,
new Tuple[] {
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE,
FIXED_HBAR_FEE_TUPLE
},
new Tuple[] {
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE,
ROYALTY_FEE_TUPLE
}))
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
setConfiguration();

final var error =
assertThrows(HandleException.class, () -> subject.decodeUpdateNonFungibleTokenCustomFees(attempt));
assertEquals(ResponseCodeEnum.CUSTOM_FEES_LIST_TOO_LONG, error.getStatus());
}

@Test
void decodeValidUpdateNonFungibleTokenCustomRoyaltyWithFallbackFeesRequest() {
// Given a valid encoded update token custom fees request
Expand All @@ -331,6 +439,7 @@ void decodeValidUpdateNonFungibleTokenCustomRoyaltyWithFallbackFeesRequest() {
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateNonFungibleTokenCustomFees(attempt);
Expand All @@ -354,6 +463,7 @@ void decodeValidUpdateNonFungibleTokenCustomRoyaltyWithHbarFallbackFeesRequest()
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
given(addressIdConverter.convert(OWNER_HEADLONG_ADDRESS)).willReturn(OWNER_ID);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateNonFungibleTokenCustomFees(attempt);
Expand All @@ -375,6 +485,7 @@ void decodeValidUpdateFungibleTokenCustomResetFeesRequest() {
EMPTY_TOKEN_FEES_TUPLE_ARR))
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateFungibleTokenCustomFees(attempt);
Expand All @@ -396,6 +507,7 @@ void decodeValidUpdateNonFungibleTokenCustomResetFeesRequest() {
EMPTY_TOKEN_FEES_TUPLE_ARR))
.toArray();
given(attempt.inputBytes()).willReturn(encoded);
setConfiguration();

// When decoding the request
final var body = subject.decodeUpdateNonFungibleTokenCustomFees(attempt);
Expand All @@ -405,4 +517,10 @@ void decodeValidUpdateNonFungibleTokenCustomResetFeesRequest() {
assertEquals(updateTokenCustomFees.tokenId(), NON_FUNGIBLE_TOKEN_ID);
assertEquals(updateTokenCustomFees.customFees().size(), 0);
}

private void setConfiguration() {
given(attempt.configuration()).willReturn(configuration);
given(configuration.getConfigData(TokensConfig.class)).willReturn(tokensConfig);
given(tokensConfig.maxCustomFeesAllowed()).willReturn(10);
}
}
10 changes: 10 additions & 0 deletions ...t/exec/systemcontracts/hts/updatetokencustomfees/UpdateTokenCustomFeesTranslatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.hedera.node.app.service.contract.impl.hevm.HederaWorldUpdater;
import com.hedera.node.app.service.contract.impl.test.exec.systemcontracts.common.CallTestBase;
import com.hedera.node.config.data.ContractsConfig;
import com.hedera.node.config.data.TokensConfig;
import com.swirlds.config.api.Configuration;
import org.apache.tuweni.bytes.Bytes;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -70,6 +71,9 @@ class UpdateTokenCustomFeesTranslatorTest extends CallTestBase {
@Mock
private ContractsConfig contractsConfig;

@Mock
private TokensConfig tokensConfig;

@Mock
private HederaWorldUpdater.Enhancement enhancement;

Expand Down Expand Up @@ -161,6 +165,9 @@ void callFromFungibleTest() {
given(addressIdConverter.convert(any())).willReturn(OWNER_ID);
given(attempt.defaultVerificationStrategy()).willReturn(verificationStrategy);
given(attempt.systemContractGasCalculator()).willReturn(gasCalculator);
given(attempt.configuration()).willReturn(configuration);
given(configuration.getConfigData(TokensConfig.class)).willReturn(tokensConfig);
given(tokensConfig.maxCustomFeesAllowed()).willReturn(10);

final var call = subject.callFrom(attempt);
assertThat(call).isInstanceOf(DispatchForResponseCodeHtsCall.class);
Expand Down Expand Up @@ -192,6 +199,9 @@ void callFromNonFungibleTest() {
given(addressIdConverter.convert(any())).willReturn(OWNER_ID);
given(attempt.defaultVerificationStrategy()).willReturn(verificationStrategy);
given(attempt.systemContractGasCalculator()).willReturn(gasCalculator);
given(attempt.configuration()).willReturn(configuration);
given(configuration.getConfigData(TokensConfig.class)).willReturn(tokensConfig);
given(tokensConfig.maxCustomFeesAllowed()).willReturn(10);

final var call = subject.callFrom(attempt);
assertThat(call).isInstanceOf(DispatchForResponseCodeHtsCall.class);
Expand Down
7 changes: 3 additions & 4 deletions ...n/java/com/hedera/services/bdd/suites/contract/precompile/UpdateTokenFeeScheduleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import static com.hedera.services.bdd.spec.utilops.UtilVerbs.overriding;
import static com.hedera.services.bdd.suites.HapiSuite.ONE_HUNDRED_HBARS;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.CONTRACT_REVERT_EXECUTED;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.CUSTOM_FEES_LIST_TOO_LONG;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.CUSTOM_FEE_DENOMINATION_MUST_BE_FUNGIBLE_COMMON;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.CUSTOM_FEE_MUST_BE_POSITIVE;
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.CUSTOM_SCHEDULE_ALREADY_HAS_NO_FEES;
Expand Down Expand Up @@ -394,13 +393,13 @@ public Stream<DynamicTest> updateTokenFeesAboveMaxAllowed() {
overriding("tokens.maxCustomFeesAllowed", "10"),
updateTokenFeeSchedules
.call("updateFungibleFixedHbarFees", fungibleToken, 11, 10L, feeCollector)
.andAssert(txn -> txn.hasKnownStatuses(CONTRACT_REVERT_EXECUTED, CUSTOM_FEES_LIST_TOO_LONG)),
.andAssert(txn -> txn.hasKnownStatus(CONTRACT_REVERT_EXECUTED)),
updateTokenFeeSchedules
.call("updateFungibleFractionalFees", feeToken, 11, 1L, 10L, false, feeCollector)
.andAssert(txn -> txn.hasKnownStatuses(CONTRACT_REVERT_EXECUTED, CUSTOM_FEES_LIST_TOO_LONG)),
.andAssert(txn -> txn.hasKnownStatus(CONTRACT_REVERT_EXECUTED)),
updateTokenFeeSchedules
.call("updateNonFungibleRoyaltyFees", nonFungibleToken, 11, 1L, 10L, feeCollector)
.andAssert(txn -> txn.hasKnownStatuses(CONTRACT_REVERT_EXECUTED, CUSTOM_FEES_LIST_TOO_LONG)));
.andAssert(txn -> txn.hasKnownStatus(CONTRACT_REVERT_EXECUTED)));
}

@Order(21)
Expand Down

0 comments on commit 6c987c2

Please sign in to comment.