Skip to content

Commit 84100ee

Browse files
overcatovercat
authored
fix: improve XDR decoding security and correctness. (#759)
 - Add decoding depth limit to prevent stack overflow (default: 200) - Add input length tracking to prevent DoS via oversized allocations - Validate variable-length array/opaque/string sizes before allocation - Validate variable-length types don't exceed declared max size - Validate fixed-length opaque/array sizes match declared size - Fix short read handling for opaque/string with proper padding - Remove incorrect auto-padding from read(byte[], int, int) - Reject unknown union discriminant values when no default arm - Validate boolean/optional flags are strictly 0 or 1 per RFC 4506 - Fix EOF handling in single-byte read - Deprecate unsafe readIntArray/readFloatArray/readDoubleArray methods
1 parent 028b666 commit 84100ee

404 files changed

Lines changed: 7854 additions & 1670 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,18 @@
1010
- fix: use `StandardCharsets.UTF_8` explicitly when converting byte arrays to strings to ensure consistent behavior across different platforms.
1111
- refactor: use static initialization for `GsonSingleton` to ensure thread safety.
1212
- fix: use `commons-codec` for hex encoding/decoding in `Util` class to properly validate input and throw clear exceptions for invalid hex strings.
13+
- fix: improve XDR decoding security and correctness.
14+
- Add decoding depth limit to prevent stack overflow (default: 200)
15+
- Add input length tracking to prevent DoS via oversized allocations
16+
- Validate variable-length array/opaque/string sizes before allocation
17+
- Validate variable-length types don't exceed declared max size
18+
- Validate fixed-length opaque/array sizes match declared size
19+
- Fix short read handling for opaque/string with proper padding
20+
- Remove incorrect auto-padding from read(byte[], int, int)
21+
- Reject unknown union discriminant values when no default arm
22+
- Validate boolean/optional flags are strictly 0 or 1 per RFC 4506
23+
- Fix EOF handling in single-byte read
24+
- Deprecate unsafe readIntArray/readFloatArray/readDoubleArray methods
1325

1426
## 2.2.1
1527

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ xdr/Stellar-contract-config-setting.x \
1414
xdr/Stellar-exporter.x
1515

1616
# xdrgen commit to use, see https://github.com/stellar/xdrgen
17-
XDRGEN_COMMIT=6b98787dcbb2b6407880adec5f74c6d88975ab0f
17+
XDRGEN_COMMIT=621d042b824f67ac65cc53d0e5e381e24aed4583
1818
# stellar-xdr commit to use, see https://github.com/stellar/stellar-xdr
1919
XDR_COMMIT=4b7a2ef7931ab2ca2499be68d849f38190b443ca
2020

‎src/main/java/org/stellar/sdk/StrKey.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package org.stellar.sdk;
22

3-
import java.io.ByteArrayInputStream;
43
import java.io.ByteArrayOutputStream;
54
import java.io.IOException;
65
import java.math.BigInteger;
@@ -18,7 +17,6 @@
1817
import org.stellar.sdk.xdr.PublicKeyType;
1918
import org.stellar.sdk.xdr.Uint256;
2019
import org.stellar.sdk.xdr.Uint64;
21-
import org.stellar.sdk.xdr.XdrDataInputStream;
2220
import org.stellar.sdk.xdr.XdrUnsignedHyperInteger;
2321

2422
/**
@@ -457,15 +455,12 @@ public static MuxedAccount encodeToXDRMuxedAccount(String data) {
457455
}
458456
break;
459457
case MED25519_PUBLIC_KEY:
460-
XdrDataInputStream input =
461-
new XdrDataInputStream(
462-
new ByteArrayInputStream(
463-
decodeCheck(VersionByte.MED25519_PUBLIC_KEY, data.toCharArray())));
458+
byte[] input = decodeCheck(VersionByte.MED25519_PUBLIC_KEY, data.toCharArray());
464459
muxed.setDiscriminant(CryptoKeyType.KEY_TYPE_MUXED_ED25519);
465460
MuxedAccount.MuxedAccountMed25519 med = new MuxedAccount.MuxedAccountMed25519();
466461
try {
467-
med.setEd25519(Uint256.decode(input));
468-
med.setId(new Uint64(XdrUnsignedHyperInteger.decode(input)));
462+
med.setEd25519(Uint256.fromXdrByteArray(Arrays.copyOfRange(input, 0, 32)));
463+
med.setId(Uint64.fromXdrByteArray(Arrays.copyOfRange(input, 32, 40)));
469464
} catch (IOException e) {
470465
throw new IllegalArgumentException("invalid address: " + data, e);
471466
}

‎src/main/java/org/stellar/sdk/xdr/AccountEntry.java‎

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -76,35 +76,60 @@ public void encode(XdrDataOutputStream stream) throws IOException {
7676
homeDomain.encode(stream);
7777
thresholds.encode(stream);
7878
int signersSize = getSigners().length;
79+
if (signersSize > 20) {
80+
throw new IOException("signers size " + signersSize + " exceeds max size 20");
81+
}
7982
stream.writeInt(signersSize);
8083
for (int i = 0; i < signersSize; i++) {
8184
signers[i].encode(stream);
8285
}
8386
ext.encode(stream);
8487
}
8588

86-
public static AccountEntry decode(XdrDataInputStream stream) throws IOException {
89+
public static AccountEntry decode(XdrDataInputStream stream, int maxDepth) throws IOException {
90+
if (maxDepth <= 0) {
91+
throw new IOException("Maximum decoding depth reached");
92+
}
93+
maxDepth -= 1;
8794
AccountEntry decodedAccountEntry = new AccountEntry();
88-
decodedAccountEntry.accountID = AccountID.decode(stream);
89-
decodedAccountEntry.balance = Int64.decode(stream);
90-
decodedAccountEntry.seqNum = SequenceNumber.decode(stream);
91-
decodedAccountEntry.numSubEntries = Uint32.decode(stream);
92-
int inflationDestPresent = stream.readInt();
93-
if (inflationDestPresent != 0) {
94-
decodedAccountEntry.inflationDest = AccountID.decode(stream);
95+
decodedAccountEntry.accountID = AccountID.decode(stream, maxDepth);
96+
decodedAccountEntry.balance = Int64.decode(stream, maxDepth);
97+
decodedAccountEntry.seqNum = SequenceNumber.decode(stream, maxDepth);
98+
decodedAccountEntry.numSubEntries = Uint32.decode(stream, maxDepth);
99+
boolean inflationDestPresent = stream.readXdrBoolean();
100+
if (inflationDestPresent) {
101+
decodedAccountEntry.inflationDest = AccountID.decode(stream, maxDepth);
95102
}
96-
decodedAccountEntry.flags = Uint32.decode(stream);
97-
decodedAccountEntry.homeDomain = String32.decode(stream);
98-
decodedAccountEntry.thresholds = Thresholds.decode(stream);
103+
decodedAccountEntry.flags = Uint32.decode(stream, maxDepth);
104+
decodedAccountEntry.homeDomain = String32.decode(stream, maxDepth);
105+
decodedAccountEntry.thresholds = Thresholds.decode(stream, maxDepth);
99106
int signersSize = stream.readInt();
107+
if (signersSize < 0) {
108+
throw new IOException("signers size " + signersSize + " is negative");
109+
}
110+
if (signersSize > 20) {
111+
throw new IOException("signers size " + signersSize + " exceeds max size 20");
112+
}
113+
int signersRemainingInputLen = stream.getRemainingInputLen();
114+
if (signersRemainingInputLen >= 0 && signersRemainingInputLen < signersSize) {
115+
throw new IOException(
116+
"signers size "
117+
+ signersSize
118+
+ " exceeds remaining input length "
119+
+ signersRemainingInputLen);
120+
}
100121
decodedAccountEntry.signers = new Signer[signersSize];
101122
for (int i = 0; i < signersSize; i++) {
102-
decodedAccountEntry.signers[i] = Signer.decode(stream);
123+
decodedAccountEntry.signers[i] = Signer.decode(stream, maxDepth);
103124
}
104-
decodedAccountEntry.ext = AccountEntryExt.decode(stream);
125+
decodedAccountEntry.ext = AccountEntryExt.decode(stream, maxDepth);
105126
return decodedAccountEntry;
106127
}
107128

129+
public static AccountEntry decode(XdrDataInputStream stream) throws IOException {
130+
return decode(stream, XdrDataInputStream.DEFAULT_MAX_DEPTH);
131+
}
132+
108133
public static AccountEntry fromXdrBase64(String xdr) throws IOException {
109134
byte[] bytes = Base64Factory.getInstance().decode(xdr);
110135
return fromXdrByteArray(bytes);
@@ -113,6 +138,7 @@ public static AccountEntry fromXdrBase64(String xdr) throws IOException {
113138
public static AccountEntry fromXdrByteArray(byte[] xdr) throws IOException {
114139
ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(xdr);
115140
XdrDataInputStream xdrDataInputStream = new XdrDataInputStream(byteArrayInputStream);
141+
xdrDataInputStream.setMaxInputLen(xdr.length);
116142
return decode(xdrDataInputStream);
117143
}
118144

@@ -148,20 +174,31 @@ public void encode(XdrDataOutputStream stream) throws IOException {
148174
}
149175
}
150176

151-
public static AccountEntryExt decode(XdrDataInputStream stream) throws IOException {
177+
public static AccountEntryExt decode(XdrDataInputStream stream, int maxDepth)
178+
throws IOException {
179+
if (maxDepth <= 0) {
180+
throw new IOException("Maximum decoding depth reached");
181+
}
182+
maxDepth -= 1;
152183
AccountEntryExt decodedAccountEntryExt = new AccountEntryExt();
153184
Integer discriminant = stream.readInt();
154185
decodedAccountEntryExt.setDiscriminant(discriminant);
155186
switch (decodedAccountEntryExt.getDiscriminant()) {
156187
case 0:
157188
break;
158189
case 1:
159-
decodedAccountEntryExt.v1 = AccountEntryExtensionV1.decode(stream);
190+
decodedAccountEntryExt.v1 = AccountEntryExtensionV1.decode(stream, maxDepth);
160191
break;
192+
default:
193+
throw new IOException("Unknown discriminant value: " + discriminant);
161194
}
162195
return decodedAccountEntryExt;
163196
}
164197

198+
public static AccountEntryExt decode(XdrDataInputStream stream) throws IOException {
199+
return decode(stream, XdrDataInputStream.DEFAULT_MAX_DEPTH);
200+
}
201+
165202
public static AccountEntryExt fromXdrBase64(String xdr) throws IOException {
166203
byte[] bytes = Base64Factory.getInstance().decode(xdr);
167204
return fromXdrByteArray(bytes);
@@ -170,6 +207,7 @@ public static AccountEntryExt fromXdrBase64(String xdr) throws IOException {
170207
public static AccountEntryExt fromXdrByteArray(byte[] xdr) throws IOException {
171208
ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(xdr);
172209
XdrDataInputStream xdrDataInputStream = new XdrDataInputStream(byteArrayInputStream);
210+
xdrDataInputStream.setMaxInputLen(xdr.length);
173211
return decode(xdrDataInputStream);
174212
}
175213
}

‎src/main/java/org/stellar/sdk/xdr/AccountEntryExtensionV1.java‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,22 @@ public void encode(XdrDataOutputStream stream) throws IOException {
4343
ext.encode(stream);
4444
}
4545

46-
public static AccountEntryExtensionV1 decode(XdrDataInputStream stream) throws IOException {
46+
public static AccountEntryExtensionV1 decode(XdrDataInputStream stream, int maxDepth)
47+
throws IOException {
48+
if (maxDepth <= 0) {
49+
throw new IOException("Maximum decoding depth reached");
50+
}
51+
maxDepth -= 1;
4752
AccountEntryExtensionV1 decodedAccountEntryExtensionV1 = new AccountEntryExtensionV1();
48-
decodedAccountEntryExtensionV1.liabilities = Liabilities.decode(stream);
49-
decodedAccountEntryExtensionV1.ext = AccountEntryExtensionV1Ext.decode(stream);
53+
decodedAccountEntryExtensionV1.liabilities = Liabilities.decode(stream, maxDepth);
54+
decodedAccountEntryExtensionV1.ext = AccountEntryExtensionV1Ext.decode(stream, maxDepth);
5055
return decodedAccountEntryExtensionV1;
5156
}
5257

58+
public static AccountEntryExtensionV1 decode(XdrDataInputStream stream) throws IOException {
59+
return decode(stream, XdrDataInputStream.DEFAULT_MAX_DEPTH);
60+
}
61+
5362
public static AccountEntryExtensionV1 fromXdrBase64(String xdr) throws IOException {
5463
byte[] bytes = Base64Factory.getInstance().decode(xdr);
5564
return fromXdrByteArray(bytes);
@@ -58,6 +67,7 @@ public static AccountEntryExtensionV1 fromXdrBase64(String xdr) throws IOExcepti
5867
public static AccountEntryExtensionV1 fromXdrByteArray(byte[] xdr) throws IOException {
5968
ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(xdr);
6069
XdrDataInputStream xdrDataInputStream = new XdrDataInputStream(byteArrayInputStream);
70+
xdrDataInputStream.setMaxInputLen(xdr.length);
6171
return decode(xdrDataInputStream);
6272
}
6373

@@ -93,7 +103,12 @@ public void encode(XdrDataOutputStream stream) throws IOException {
93103
}
94104
}
95105

96-
public static AccountEntryExtensionV1Ext decode(XdrDataInputStream stream) throws IOException {
106+
public static AccountEntryExtensionV1Ext decode(XdrDataInputStream stream, int maxDepth)
107+
throws IOException {
108+
if (maxDepth <= 0) {
109+
throw new IOException("Maximum decoding depth reached");
110+
}
111+
maxDepth -= 1;
97112
AccountEntryExtensionV1Ext decodedAccountEntryExtensionV1Ext =
98113
new AccountEntryExtensionV1Ext();
99114
Integer discriminant = stream.readInt();
@@ -102,12 +117,18 @@ public static AccountEntryExtensionV1Ext decode(XdrDataInputStream stream) throw
102117
case 0:
103118
break;
104119
case 2:
105-
decodedAccountEntryExtensionV1Ext.v2 = AccountEntryExtensionV2.decode(stream);
120+
decodedAccountEntryExtensionV1Ext.v2 = AccountEntryExtensionV2.decode(stream, maxDepth);
106121
break;
122+
default:
123+
throw new IOException("Unknown discriminant value: " + discriminant);
107124
}
108125
return decodedAccountEntryExtensionV1Ext;
109126
}
110127

128+
public static AccountEntryExtensionV1Ext decode(XdrDataInputStream stream) throws IOException {
129+
return decode(stream, XdrDataInputStream.DEFAULT_MAX_DEPTH);
130+
}
131+
111132
public static AccountEntryExtensionV1Ext fromXdrBase64(String xdr) throws IOException {
112133
byte[] bytes = Base64Factory.getInstance().decode(xdr);
113134
return fromXdrByteArray(bytes);
@@ -116,6 +137,7 @@ public static AccountEntryExtensionV1Ext fromXdrBase64(String xdr) throws IOExce
116137
public static AccountEntryExtensionV1Ext fromXdrByteArray(byte[] xdr) throws IOException {
117138
ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(xdr);
118139
XdrDataInputStream xdrDataInputStream = new XdrDataInputStream(byteArrayInputStream);
140+
xdrDataInputStream.setMaxInputLen(xdr.length);
119141
return decode(xdrDataInputStream);
120142
}
121143
}

‎src/main/java/org/stellar/sdk/xdr/AccountEntryExtensionV2.java‎

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,27 +46,57 @@ public void encode(XdrDataOutputStream stream) throws IOException {
4646
numSponsored.encode(stream);
4747
numSponsoring.encode(stream);
4848
int signerSponsoringIDsSize = getSignerSponsoringIDs().length;
49+
if (signerSponsoringIDsSize > 20) {
50+
throw new IOException(
51+
"signerSponsoringIDs size " + signerSponsoringIDsSize + " exceeds max size 20");
52+
}
4953
stream.writeInt(signerSponsoringIDsSize);
5054
for (int i = 0; i < signerSponsoringIDsSize; i++) {
5155
signerSponsoringIDs[i].encode(stream);
5256
}
5357
ext.encode(stream);
5458
}
5559

56-
public static AccountEntryExtensionV2 decode(XdrDataInputStream stream) throws IOException {
60+
public static AccountEntryExtensionV2 decode(XdrDataInputStream stream, int maxDepth)
61+
throws IOException {
62+
if (maxDepth <= 0) {
63+
throw new IOException("Maximum decoding depth reached");
64+
}
65+
maxDepth -= 1;
5766
AccountEntryExtensionV2 decodedAccountEntryExtensionV2 = new AccountEntryExtensionV2();
58-
decodedAccountEntryExtensionV2.numSponsored = Uint32.decode(stream);
59-
decodedAccountEntryExtensionV2.numSponsoring = Uint32.decode(stream);
67+
decodedAccountEntryExtensionV2.numSponsored = Uint32.decode(stream, maxDepth);
68+
decodedAccountEntryExtensionV2.numSponsoring = Uint32.decode(stream, maxDepth);
6069
int signerSponsoringIDsSize = stream.readInt();
70+
if (signerSponsoringIDsSize < 0) {
71+
throw new IOException("signerSponsoringIDs size " + signerSponsoringIDsSize + " is negative");
72+
}
73+
if (signerSponsoringIDsSize > 20) {
74+
throw new IOException(
75+
"signerSponsoringIDs size " + signerSponsoringIDsSize + " exceeds max size 20");
76+
}
77+
int signerSponsoringIDsRemainingInputLen = stream.getRemainingInputLen();
78+
if (signerSponsoringIDsRemainingInputLen >= 0
79+
&& signerSponsoringIDsRemainingInputLen < signerSponsoringIDsSize) {
80+
throw new IOException(
81+
"signerSponsoringIDs size "
82+
+ signerSponsoringIDsSize
83+
+ " exceeds remaining input length "
84+
+ signerSponsoringIDsRemainingInputLen);
85+
}
6186
decodedAccountEntryExtensionV2.signerSponsoringIDs =
6287
new SponsorshipDescriptor[signerSponsoringIDsSize];
6388
for (int i = 0; i < signerSponsoringIDsSize; i++) {
64-
decodedAccountEntryExtensionV2.signerSponsoringIDs[i] = SponsorshipDescriptor.decode(stream);
89+
decodedAccountEntryExtensionV2.signerSponsoringIDs[i] =
90+
SponsorshipDescriptor.decode(stream, maxDepth);
6591
}
66-
decodedAccountEntryExtensionV2.ext = AccountEntryExtensionV2Ext.decode(stream);
92+
decodedAccountEntryExtensionV2.ext = AccountEntryExtensionV2Ext.decode(stream, maxDepth);
6793
return decodedAccountEntryExtensionV2;
6894
}
6995

96+
public static AccountEntryExtensionV2 decode(XdrDataInputStream stream) throws IOException {
97+
return decode(stream, XdrDataInputStream.DEFAULT_MAX_DEPTH);
98+
}
99+
70100
public static AccountEntryExtensionV2 fromXdrBase64(String xdr) throws IOException {
71101
byte[] bytes = Base64Factory.getInstance().decode(xdr);
72102
return fromXdrByteArray(bytes);
@@ -75,6 +105,7 @@ public static AccountEntryExtensionV2 fromXdrBase64(String xdr) throws IOExcepti
75105
public static AccountEntryExtensionV2 fromXdrByteArray(byte[] xdr) throws IOException {
76106
ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(xdr);
77107
XdrDataInputStream xdrDataInputStream = new XdrDataInputStream(byteArrayInputStream);
108+
xdrDataInputStream.setMaxInputLen(xdr.length);
78109
return decode(xdrDataInputStream);
79110
}
80111

@@ -110,7 +141,12 @@ public void encode(XdrDataOutputStream stream) throws IOException {
110141
}
111142
}
112143

113-
public static AccountEntryExtensionV2Ext decode(XdrDataInputStream stream) throws IOException {
144+
public static AccountEntryExtensionV2Ext decode(XdrDataInputStream stream, int maxDepth)
145+
throws IOException {
146+
if (maxDepth <= 0) {
147+
throw new IOException("Maximum decoding depth reached");
148+
}
149+
maxDepth -= 1;
114150
AccountEntryExtensionV2Ext decodedAccountEntryExtensionV2Ext =
115151
new AccountEntryExtensionV2Ext();
116152
Integer discriminant = stream.readInt();
@@ -119,12 +155,18 @@ public static AccountEntryExtensionV2Ext decode(XdrDataInputStream stream) throw
119155
case 0:
120156
break;
121157
case 3:
122-
decodedAccountEntryExtensionV2Ext.v3 = AccountEntryExtensionV3.decode(stream);
158+
decodedAccountEntryExtensionV2Ext.v3 = AccountEntryExtensionV3.decode(stream, maxDepth);
123159
break;
160+
default:
161+
throw new IOException("Unknown discriminant value: " + discriminant);
124162
}
125163
return decodedAccountEntryExtensionV2Ext;
126164
}
127165

166+
public static AccountEntryExtensionV2Ext decode(XdrDataInputStream stream) throws IOException {
167+
return decode(stream, XdrDataInputStream.DEFAULT_MAX_DEPTH);
168+
}
169+
128170
public static AccountEntryExtensionV2Ext fromXdrBase64(String xdr) throws IOException {
129171
byte[] bytes = Base64Factory.getInstance().decode(xdr);
130172
return fromXdrByteArray(bytes);
@@ -133,6 +175,7 @@ public static AccountEntryExtensionV2Ext fromXdrBase64(String xdr) throws IOExce
133175
public static AccountEntryExtensionV2Ext fromXdrByteArray(byte[] xdr) throws IOException {
134176
ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(xdr);
135177
XdrDataInputStream xdrDataInputStream = new XdrDataInputStream(byteArrayInputStream);
178+
xdrDataInputStream.setMaxInputLen(xdr.length);
136179
return decode(xdrDataInputStream);
137180
}
138181
}

0 commit comments

Comments
 (0)