Skip to content

Commit 8292fec

Browse files
daniellehrnermacfarla
authored andcommitted
Fix 7702 signature bound checks (besu-eth#7641)
* 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> Signed-off-by: Wolmin <lamonos123@gmail.com>
1 parent 07025b8 commit 8292fec

File tree

8 files changed

+193
-3
lines changed

8 files changed

+193
-3
lines changed

.github/workflows/acceptance-tests.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ concurrency:
1111
cancel-in-progress: true
1212

1313
env:
14-
GRADLE_OPTS: "-Xmx6g"
14+
GRADLE_OPTS: "-Xmx7g"
1515
total-runners: 12
1616

1717
jobs:

crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/AbstractSECP256.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,12 @@ public SECPSignature createSignature(final BigInteger r, final BigInteger s, fin
212212
return SECPSignature.create(r, s, recId, curveOrder);
213213
}
214214

215+
@Override
216+
public CodeDelegationSignature createCodeDelegationSignature(
217+
final BigInteger r, final BigInteger s, final long yParity) {
218+
return CodeDelegationSignature.create(r, s, yParity);
219+
}
220+
215221
@Override
216222
public SECPSignature decodeSignature(final Bytes bytes) {
217223
return SECPSignature.decode(bytes, curveOrder);
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* Copyright contributors to Hyperledger Besu.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
5+
* the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
10+
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
11+
* specific language governing permissions and limitations under the License.
12+
*
13+
* SPDX-License-Identifier: Apache-2.0
14+
*/
15+
package org.hyperledger.besu.crypto;
16+
17+
import static com.google.common.base.Preconditions.checkNotNull;
18+
19+
import java.math.BigInteger;
20+
21+
/** Secp signature with code delegation. */
22+
public class CodeDelegationSignature extends SECPSignature {
23+
private static final BigInteger TWO_POW_256 = BigInteger.TWO.pow(256);
24+
25+
/**
26+
* Instantiates a new SECPSignature.
27+
*
28+
* @param r the r part of the signature
29+
* @param s the s part of the signature
30+
* @param yParity the parity of the y coordinate of the public key
31+
*/
32+
public CodeDelegationSignature(final BigInteger r, final BigInteger s, final byte yParity) {
33+
super(r, s, yParity);
34+
}
35+
36+
/**
37+
* Create a new CodeDelegationSignature.
38+
*
39+
* @param r the r part of the signature
40+
* @param s the s part of the signature
41+
* @param yParity the parity of the y coordinate of the public key
42+
* @return the new CodeDelegationSignature
43+
*/
44+
public static CodeDelegationSignature create(
45+
final BigInteger r, final BigInteger s, final long yParity) {
46+
checkNotNull(r);
47+
checkNotNull(s);
48+
49+
if (r.compareTo(TWO_POW_256) >= 0) {
50+
throw new IllegalArgumentException("Invalid 'r' value, should be < 2^256 but got " + r);
51+
}
52+
53+
if (s.compareTo(TWO_POW_256) >= 0) {
54+
throw new IllegalArgumentException("Invalid 's' value, should be < 2^256 but got " + s);
55+
}
56+
57+
return new CodeDelegationSignature(r, s, (byte) yParity);
58+
}
59+
}

crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithm.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,17 @@ Optional<SECPPublicKey> recoverPublicKeyFromSignature(
215215
*/
216216
SECPSignature createSignature(final BigInteger r, final BigInteger s, final byte recId);
217217

218+
/**
219+
* Create code delegation signature which have different bounds than transaction signatures.
220+
*
221+
* @param r the r part of the signature
222+
* @param s the s part of the signature
223+
* @param yParity the parity of the y coordinate of the public key
224+
* @return the code delegation signature
225+
*/
226+
CodeDelegationSignature createCodeDelegationSignature(
227+
final BigInteger r, final BigInteger s, final long yParity);
228+
218229
/**
219230
* Decode secp signature.
220231
*
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* Copyright contributors to Hyperledger Besu.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
5+
* the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
10+
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
11+
* specific language governing permissions and limitations under the License.
12+
*
13+
* SPDX-License-Identifier: Apache-2.0
14+
*/
15+
package org.hyperledger.besu.crypto;
16+
17+
import static org.assertj.core.api.Assertions.assertThat;
18+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
19+
20+
import java.math.BigInteger;
21+
22+
import org.junit.jupiter.api.Test;
23+
24+
class CodeDelegationSignatureTest {
25+
26+
private static final BigInteger TWO_POW_256 = BigInteger.valueOf(2).pow(256);
27+
28+
@Test
29+
void testValidInputs() {
30+
BigInteger r = BigInteger.ONE;
31+
BigInteger s = BigInteger.TEN;
32+
long yParity = 1L;
33+
34+
CodeDelegationSignature result = CodeDelegationSignature.create(r, s, yParity);
35+
36+
assertThat(r).isEqualTo(result.getR());
37+
assertThat(s).isEqualTo(result.getS());
38+
assertThat((byte) yParity).isEqualTo(result.getRecId());
39+
}
40+
41+
@Test
42+
void testNullRValue() {
43+
BigInteger s = BigInteger.TEN;
44+
long yParity = 0L;
45+
46+
assertThatExceptionOfType(NullPointerException.class)
47+
.isThrownBy(() -> CodeDelegationSignature.create(null, s, yParity));
48+
}
49+
50+
@Test
51+
void testNullSValue() {
52+
BigInteger r = BigInteger.ONE;
53+
long yParity = 0L;
54+
55+
assertThatExceptionOfType(NullPointerException.class)
56+
.isThrownBy(() -> CodeDelegationSignature.create(r, null, yParity));
57+
}
58+
59+
@Test
60+
void testRValueExceedsTwoPow256() {
61+
BigInteger r = TWO_POW_256;
62+
BigInteger s = BigInteger.TEN;
63+
long yParity = 0L;
64+
65+
assertThatExceptionOfType(IllegalArgumentException.class)
66+
.isThrownBy(() -> CodeDelegationSignature.create(r, s, yParity))
67+
.withMessageContainingAll("Invalid 'r' value, should be < 2^256");
68+
}
69+
70+
@Test
71+
void testSValueExceedsTwoPow256() {
72+
BigInteger r = BigInteger.ONE;
73+
BigInteger s = TWO_POW_256;
74+
long yParity = 0L;
75+
76+
assertThatExceptionOfType(IllegalArgumentException.class)
77+
.isThrownBy(() -> CodeDelegationSignature.create(r, s, yParity))
78+
.withMessageContainingAll("Invalid 's' value, should be < 2^256");
79+
}
80+
81+
@Test
82+
void testValidYParityZero() {
83+
BigInteger r = BigInteger.ONE;
84+
BigInteger s = BigInteger.TEN;
85+
long yParity = 0L;
86+
87+
CodeDelegationSignature result = CodeDelegationSignature.create(r, s, yParity);
88+
89+
assertThat(r).isEqualTo(result.getR());
90+
assertThat(s).isEqualTo(result.getS());
91+
assertThat((byte) yParity).isEqualTo(result.getRecId());
92+
}
93+
}

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationTransactionDecoder.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,14 @@ public static CodeDelegation decodeInnerPayload(final RLPInput input) {
8181
final Address address = Address.wrap(input.readBytes());
8282
final long nonce = input.readLongScalar();
8383

84-
final byte yParity = (byte) input.readUnsignedByteScalar();
84+
final long yParity = input.readUnsignedIntScalar();
8585
final BigInteger r = input.readUInt256Scalar().toUnsignedBigInteger();
8686
final BigInteger s = input.readUInt256Scalar().toUnsignedBigInteger();
8787

8888
input.leaveList();
8989

90-
final SECPSignature signature = SIGNATURE_ALGORITHM.get().createSignature(r, s, yParity);
90+
final SECPSignature signature =
91+
SIGNATURE_ALGORITHM.get().createCodeDelegationSignature(r, s, yParity);
9192

9293
return new org.hyperledger.besu.ethereum.core.CodeDelegation(
9394
chainId, address, nonce, signature);

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@
5252
*/
5353
public class MainnetTransactionValidator implements TransactionValidator {
5454

55+
public static final BigInteger TWO_POW_256 = BigInteger.TWO.pow(256);
56+
5557
private final GasCalculator gasCalculator;
5658
private final GasLimitCalculator gasLimitCalculator;
5759
private final FeeMarket feeMarket;
@@ -163,6 +165,12 @@ private static ValidationResult<TransactionInvalidReason> validateCodeDelegation
163165
.map(
164166
codeDelegations -> {
165167
for (CodeDelegation codeDelegation : codeDelegations) {
168+
if (codeDelegation.chainId().compareTo(TWO_POW_256) >= 0) {
169+
throw new IllegalArgumentException(
170+
"Invalid 'chainId' value, should be < 2^256 but got "
171+
+ codeDelegation.chainId());
172+
}
173+
166174
if (codeDelegation.signature().getS().compareTo(halfCurveOrder) > 0) {
167175
return ValidationResult.invalid(
168176
TransactionInvalidReason.INVALID_SIGNATURE,

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationDecoderTest.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,16 @@ void shouldDecodeInnerPayloadWithChainIdZero() {
9999
assertThat(signature.getS().toString(16))
100100
.isEqualTo("3c8a25b2becd6e666f69803d1ae3322f2e137b7745c2c7f19da80f993ffde4df");
101101
}
102+
103+
@Test
104+
void shouldDecodeInnerPayloadWhenSignatureIsZero() {
105+
final BytesValueRLPInput input =
106+
new BytesValueRLPInput(
107+
Bytes.fromHexString(
108+
"0xdf8501a1f0ff5a947a40026a3b9a41754a95eec8c92c6b99886f440c5b808080"),
109+
true);
110+
final CodeDelegation authorization = CodeDelegationTransactionDecoder.decodeInnerPayload(input);
111+
112+
assertThat(authorization.chainId()).isEqualTo(new BigInteger("01a1f0ff5a", 16));
113+
}
102114
}

0 commit comments

Comments
 (0)