mirror of
https://github.com/ethereum/go-ethereum.git
synced 2026-07-23 05:06:43 +00:00
7a5b24894d
* qkc/serialize: port QuarkChain binary serialization (byte-compatible)
Verbatim port of github.com/QuarkChain/goquarkchain/serialize, the
reflection-based binary codec used (instead of RLP) for all slavechain
headers, blocks and storage records. Pure stdlib, zero adaptations.
Includes the original golden byte-vector tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: bound deserializeList allocation by remaining input
deserializeList allocated reflect.MakeSlice(val.Type(), vlen, vlen) from the
wire-supplied length before reading any element bytes, with no bound check. A
malicious P2P packet could claim a huge element count (up to ~4B with
bytesizeofslicelen:4) and force a massive allocation from a tiny packet — an
OOM/DoS vector, since this codec decodes blocks/headers straight from peers.
Reject a length larger than the remaining buffer (every list element consumes
at least one byte, so such a length is always malformed and would fail below
anyway). This does not change the decoding of any valid input; it is an
intentional hardening divergence from the verbatim goquarkchain port, which has
the same unbounded allocation.
Addresses review feedback from @qzhodl.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: reject negative big.Int in fixed-size Uint128/Uint256
serializeFixSizeBigUint built the fixed-width bytes from big.Int.Bytes(),
which returns the absolute value. A negative value therefore serialized
identically to its positive counterpart (e.g. -1 as +1), silently changing
it. Reject negatives, matching the variable-length serializeBigInt path which
already does so. Non-negative encodings are unchanged.
This is an intentional hardening divergence from the verbatim goquarkchain
port, whose fixed-size path has the same silent sign loss while its
variable-length path rejects negatives.
Addresses review feedback from @qzhodl.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: reject big.Int that overflows the single-byte length prefix
serializeBigInt writes the byte length as a single uint8, so a value needing
more than 255 bytes (>2040 bits) had its length silently truncated and decoded
to a different value — a non-roundtrippable encoding. Return an error when
len(bytes) > 255. Values at or under the limit are unchanged and still
round-trip.
Addresses review feedback from @qzhodl. (Same single-byte length prefix exists
in the verbatim goquarkchain port.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: reject trailing bytes in DeserializeFromBytes
DeserializeFromBytes decoded from a fresh buffer but never checked that the
buffer was fully consumed, so an input of valid data plus trailing garbage was
silently accepted. Return an error when bytes remain after a successful decode.
The lower-level Deserialize(bb, val) stays permissive so callers can stream
multiple values from one buffer.
All current callers pass an exact SerializeToBytes blob, so this only rejects
malformed/over-long input.
Addresses review feedback from @qzhodl.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: reject non-canonical nil-pointer presence markers
The ser:"nil" presence marker is written as exactly 0 (absent) or 1 (present),
but the deserializer treated any non-zero byte as present. That let distinct
byte strings (e.g. 0x05 and 0x01) decode to the same value, a non-canonical
encoding. Reject any marker other than 0 or 1.
A ported table case used a non-canonical marker (0x03) to reach a truncated-
buffer error; switch it to the canonical 0x01 so it still exercises that path,
and add TestDeserializeNilMarkerCanonical to cover the rejection directly.
Addresses review feedback from @qzhodl.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: reject uint encodings wider than the destination
The variable-length reflect.Uint path reads a 1-byte length prefix, so it can
deliver up to 255 bytes. Those bytes were folded into a uint64 accumulator with
no width check, so an encoding longer than the uint width overflowed the
accumulator and silently truncated the decoded value. Reject any encoding whose
byte count exceeds the destination width (Type().Bits()/8). Fixed-width kinds
always read exactly that many bytes, so this is a no-op for them.
Addresses review feedback from @qzhodl.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: reject zero-byte list element types at registration
The deserializeList bound (vlen <= remaining bytes) assumes every element
consumes at least one byte, but the codec can encode zero-byte elements
(struct{}, structs whose fields are all ignored/unexported, [0]T). Such a list
serializes to just its length prefix, so a valid non-empty round-trip then
failed the bound — the codec emitted bytes it refused to decode.
Reject slices of zero-byte-encoding elements symmetrically at type registration
(genTypeInfo via encodesZeroBytes), so serialize and deserialize agree and the
allocation bound stays sound for the >=1-byte element types it protects. Byte
slices and arrays are exempt (a byte is one byte; array length is fixed by the
type, not read from input). QKC has no such list types, so this changes no
valid encoding. The ported nil-slice-pointer case used struct{ uint } (embedded
unexported field => zero bytes); give it an exported field so it stays a
supported element while still exercising nil-pointer-to-slice.
Addresses review feedback from @qzhodl.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: unmark visited types on return in encodesZeroBytes
The visited set is meant to break reference cycles, but it was marked on the way
down and never unmarked, so it tracked every type ever seen rather than the
current recursion path. A struct with repeated fields of the same zero-byte type
(e.g. struct{ A empty; B empty }) then hit a stale mark on the second field and
was misclassified as non-zero, so a slice of it escaped the registration check —
serialize emitted a bare length prefix that deserialize rejected, reintroducing
the encode/decode asymmetry the check exists to prevent.
defer delete(visited, typ) scopes the set to the current path: repeated sibling
fields are evaluated independently, while a type still on the path is still
treated as a cycle. Add TestZeroByteRepeatedFieldTypeRejected pinning the
symmetric rejection.
Addresses review feedback from @syntrust.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: reject nil value in SerializeWithTags
SerializeWithTags called reflect.ValueOf(val).Type() unguarded, so an untyped
nil panicked ("reflect: call of reflect.Value.Type on zero Value") instead of
returning an error. DeserializeWithTags already rejects a nil target up front;
add the symmetric check on the serialize side. A typed nil pointer still carries
a type and continues to serialize as before.
Addresses review feedback from @syntrust.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* qkc/serialize: bound length-prefix accumulation in getLen
getLen accumulated the length prefix into a signed int with an unbounded left
shift. Once the prefix reached the platform int width (4 bytes on 32-bit, 8 on
64-bit) a high-bit-set byte landed in the sign bit and silently produced a
negative length. A negative length slipped past downstream len<=remaining checks
and then panicked (reflect.MakeSlice: negative len / slice bounds out of range)
on attacker-controlled input — confirmed with an 8-byte 0xFF prefix.
Accumulate in uint64, reject a shift that would overflow it, and reject a value
that does not fit a non-negative int. This changes no representable length (a
wide prefix encoding a small value still decodes). Hardening divergence from
goquarkchain.
Addresses review feedback from @syntrust.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
34 lines
1.1 KiB
Go
34 lines
1.1 KiB
Go
package serialize
|
|
|
|
import (
|
|
"math/big"
|
|
"testing"
|
|
)
|
|
|
|
// TestFixedSizeUintRejectsNegative guards against silent sign loss: because
|
|
// big.Int.Bytes() returns the absolute value, a negative Uint128/Uint256 would
|
|
// otherwise serialize identically to its positive counterpart. It must error
|
|
// instead.
|
|
func TestFixedSizeUintRejectsNegative(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
ser func() error
|
|
}{
|
|
{"Uint128", func() error { var w []byte; return (&Uint128{Value: big.NewInt(-1)}).Serialize(&w) }},
|
|
{"Uint256", func() error { var w []byte; return (&Uint256{Value: big.NewInt(-1)}).Serialize(&w) }},
|
|
}
|
|
for _, tc := range cases {
|
|
if err := tc.ser(); err == nil {
|
|
t.Fatalf("%s: expected an error serializing a negative value, got nil (sign silently dropped)", tc.name)
|
|
}
|
|
}
|
|
|
|
// Non-negative values still serialize to their fixed width.
|
|
var w []byte
|
|
if err := (&Uint256{Value: big.NewInt(1)}).Serialize(&w); err != nil {
|
|
t.Fatalf("positive Uint256 rejected: %v", err)
|
|
}
|
|
if len(w) != 32 {
|
|
t.Fatalf("Uint256 should serialize to 32 bytes, got %d", len(w))
|
|
}
|
|
}
|