Skip to content

Commit

Permalink
Fix 7702 signature bound checks (hyperledger#7641)
Browse files Browse the repository at this point in the history
* create separate signature class for code delegations as they have different bound checks

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

* test if increasing xmx let's failing acceptance test pass

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

* javadoc

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
  • Loading branch information
daniellehrner and macfarla authored Sep 20, 2024
1 parent 25186d3 commit cb1e36a
Show file tree
Hide file tree
Showing 8 changed files with 193 additions and 3 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/acceptance-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ concurrency:
cancel-in-progress: true

env:
GRADLE_OPTS: "-Xmx6g"
GRADLE_OPTS: "-Xmx7g"
total-runners: 12

jobs:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ public SECPSignature createSignature(final BigInteger r, final BigInteger s, fin
return SECPSignature.create(r, s, recId, curveOrder);
}

@Override
public CodeDelegationSignature createCodeDelegationSignature(
final BigInteger r, final BigInteger s, final long yParity) {
return CodeDelegationSignature.create(r, s, yParity);
}

@Override
public SECPSignature decodeSignature(final Bytes bytes) {
return SECPSignature.decode(bytes, curveOrder);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright contributors to Hyperledger Besu.
*
* 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.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.crypto;

import static com.google.common.base.Preconditions.checkNotNull;

import java.math.BigInteger;

/** Secp signature with code delegation. */
public class CodeDelegationSignature extends SECPSignature {
private static final BigInteger TWO_POW_256 = BigInteger.TWO.pow(256);

/**
* Instantiates a new SECPSignature.
*
* @param r the r part of the signature
* @param s the s part of the signature
* @param yParity the parity of the y coordinate of the public key
*/
public CodeDelegationSignature(final BigInteger r, final BigInteger s, final byte yParity) {
super(r, s, yParity);
}

/**
* Create a new CodeDelegationSignature.
*
* @param r the r part of the signature
* @param s the s part of the signature
* @param yParity the parity of the y coordinate of the public key
* @return the new CodeDelegationSignature
*/
public static CodeDelegationSignature create(
final BigInteger r, final BigInteger s, final long yParity) {
checkNotNull(r);
checkNotNull(s);

if (r.compareTo(TWO_POW_256) >= 0) {
throw new IllegalArgumentException("Invalid 'r' value, should be < 2^256 but got " + r);
}

if (s.compareTo(TWO_POW_256) >= 0) {
throw new IllegalArgumentException("Invalid 's' value, should be < 2^256 but got " + s);
}

return new CodeDelegationSignature(r, s, (byte) yParity);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,17 @@ Optional<SECPPublicKey> recoverPublicKeyFromSignature(
*/
SECPSignature createSignature(final BigInteger r, final BigInteger s, final byte recId);

/**
* Create code delegation signature which have different bounds than transaction signatures.
*
* @param r the r part of the signature
* @param s the s part of the signature
* @param yParity the parity of the y coordinate of the public key
* @return the code delegation signature
*/
CodeDelegationSignature createCodeDelegationSignature(
final BigInteger r, final BigInteger s, final long yParity);

/**
* Decode secp signature.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* Copyright contributors to Hyperledger Besu.
*
* 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.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.crypto;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;

import java.math.BigInteger;

import org.junit.jupiter.api.Test;

class CodeDelegationSignatureTest {

private static final BigInteger TWO_POW_256 = BigInteger.valueOf(2).pow(256);

@Test
void testValidInputs() {
BigInteger r = BigInteger.ONE;
BigInteger s = BigInteger.TEN;
long yParity = 1L;

CodeDelegationSignature result = CodeDelegationSignature.create(r, s, yParity);

assertThat(r).isEqualTo(result.getR());
assertThat(s).isEqualTo(result.getS());
assertThat((byte) yParity).isEqualTo(result.getRecId());
}

@Test
void testNullRValue() {
BigInteger s = BigInteger.TEN;
long yParity = 0L;

assertThatExceptionOfType(NullPointerException.class)
.isThrownBy(() -> CodeDelegationSignature.create(null, s, yParity));
}

@Test
void testNullSValue() {
BigInteger r = BigInteger.ONE;
long yParity = 0L;

assertThatExceptionOfType(NullPointerException.class)
.isThrownBy(() -> CodeDelegationSignature.create(r, null, yParity));
}

@Test
void testRValueExceedsTwoPow256() {
BigInteger r = TWO_POW_256;
BigInteger s = BigInteger.TEN;
long yParity = 0L;

assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> CodeDelegationSignature.create(r, s, yParity))
.withMessageContainingAll("Invalid 'r' value, should be < 2^256");
}

@Test
void testSValueExceedsTwoPow256() {
BigInteger r = BigInteger.ONE;
BigInteger s = TWO_POW_256;
long yParity = 0L;

assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> CodeDelegationSignature.create(r, s, yParity))
.withMessageContainingAll("Invalid 's' value, should be < 2^256");
}

@Test
void testValidYParityZero() {
BigInteger r = BigInteger.ONE;
BigInteger s = BigInteger.TEN;
long yParity = 0L;

CodeDelegationSignature result = CodeDelegationSignature.create(r, s, yParity);

assertThat(r).isEqualTo(result.getR());
assertThat(s).isEqualTo(result.getS());
assertThat((byte) yParity).isEqualTo(result.getRecId());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,14 @@ public static CodeDelegation decodeInnerPayload(final RLPInput input) {
final Address address = Address.wrap(input.readBytes());
final long nonce = input.readLongScalar();

final byte yParity = (byte) input.readUnsignedByteScalar();
final long yParity = input.readUnsignedIntScalar();
final BigInteger r = input.readUInt256Scalar().toUnsignedBigInteger();
final BigInteger s = input.readUInt256Scalar().toUnsignedBigInteger();

input.leaveList();

final SECPSignature signature = SIGNATURE_ALGORITHM.get().createSignature(r, s, yParity);
final SECPSignature signature =
SIGNATURE_ALGORITHM.get().createCodeDelegationSignature(r, s, yParity);

return new org.hyperledger.besu.ethereum.core.CodeDelegation(
chainId, address, nonce, signature);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
*/
public class MainnetTransactionValidator implements TransactionValidator {

public static final BigInteger TWO_POW_256 = BigInteger.TWO.pow(256);

private final GasCalculator gasCalculator;
private final GasLimitCalculator gasLimitCalculator;
private final FeeMarket feeMarket;
Expand Down Expand Up @@ -163,6 +165,12 @@ private static ValidationResult<TransactionInvalidReason> validateCodeDelegation
.map(
codeDelegations -> {
for (CodeDelegation codeDelegation : codeDelegations) {
if (codeDelegation.chainId().compareTo(TWO_POW_256) >= 0) {
throw new IllegalArgumentException(
"Invalid 'chainId' value, should be < 2^256 but got "
+ codeDelegation.chainId());
}

if (codeDelegation.signature().getS().compareTo(halfCurveOrder) > 0) {
return ValidationResult.invalid(
TransactionInvalidReason.INVALID_SIGNATURE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,16 @@ void shouldDecodeInnerPayloadWithChainIdZero() {
assertThat(signature.getS().toString(16))
.isEqualTo("3c8a25b2becd6e666f69803d1ae3322f2e137b7745c2c7f19da80f993ffde4df");
}

@Test
void shouldDecodeInnerPayloadWhenSignatureIsZero() {
final BytesValueRLPInput input =
new BytesValueRLPInput(
Bytes.fromHexString(
"0xdf8501a1f0ff5a947a40026a3b9a41754a95eec8c92c6b99886f440c5b808080"),
true);
final CodeDelegation authorization = CodeDelegationTransactionDecoder.decodeInnerPayload(input);

assertThat(authorization.chainId()).isEqualTo(new BigInteger("01a1f0ff5a", 16));
}
}

0 comments on commit cb1e36a

Please sign in to comment.