Detect another invalid SSZ input found through fuzzing

The first offset of an SSZ object should always have a fixed constant
value. Otherwise, some unused bytes may appear between the fixed portion
and the dynamic portion.

Please note that this fix shutds down the minimal forward compatibility
currently supported by the SSZ format (and thus, the expected behavior
must be clarified in the SSZ spec).
This commit is contained in:
Zahary Karadjov 2020-05-29 22:06:35 +03:00 committed by zah
parent 8d1fb2f3a6
commit 0bcdabfcdf
9 changed files with 28 additions and 12 deletions

View File

@ -28,6 +28,9 @@ import
# ################### Helper functions ###################################
when hasSerializationTracing:
import stew/byteutils
export
serialization, types, bytes_reader

View File

@ -225,6 +225,11 @@ func readSszValue*[T](input: openarray[byte], val: var T) {.raisesssz.} =
startOffset = readOffsetUnchecked(boundingOffsets[0])
endOffset = if boundingOffsets[1] == -1: input.len
else: readOffsetUnchecked(boundingOffsets[1])
when boundingOffsets.isFirstOffset:
if startOffset != minimallyExpectedSize:
raise newException(MalformedSszError, "SSZ object dynamic portion starts at invalid offset")
trs "VAR FIELD ", startOffset, "-", endOffset
if startOffset > endOffset:
raise newException(MalformedSszError, "SSZ field offsets are not monotonically increasing")

View File

@ -165,9 +165,10 @@ proc fieldInfos*(RecordType: type): seq[tuple[name: string,
func getFieldBoundingOffsetsImpl(RecordType: type,
fieldName: static string):
tuple[fieldOffset, nextFieldOffset: int] {.compileTime.} =
result = (-1, -1)
tuple[fieldOffset, nextFieldOffset: int, isFirstOffset: bool] {.compileTime.} =
result = (-1, -1, false)
var fieldBranchKey: string
var isFirstOffset = true
for f in fieldInfos(RecordType):
if fieldName == f.name:
@ -177,6 +178,7 @@ func getFieldBoundingOffsetsImpl(RecordType: type,
return
else:
fieldBranchKey = f.branchKey
result.isFirstOffset = isFirstOffset
elif result[0] != -1 and
f.fixedSize == 0 and
@ -185,9 +187,12 @@ func getFieldBoundingOffsetsImpl(RecordType: type,
result[1] = f.offset
return
if f.fixedSize == 0:
isFirstOffset = false
func getFieldBoundingOffsets*(RecordType: type,
fieldName: static string):
tuple[fieldOffset, nextFieldOffset: int] {.compileTime.} =
tuple[fieldOffset, nextFieldOffset: int, isFirstOffset: bool] {.compileTime.} =
## Returns the start and end offsets of a field.
##
## For fixed-size fields, the start offset points to the first

View File

@ -3,7 +3,7 @@ import strformat, confutils
const
gitRoot = thisDir() / ".."
fixturesDir = gitRoot / "vendor" / "nim-eth2-scenarios" / "tests-v0.11.2" / "mainnet" / "phase0" / "ssz_static"
fixturesDir = gitRoot / "vendor" / "nim-eth2-scenarios" / "tests-v0.11.3" / "mainnet" / "phase0" / "ssz_static"
fuzzingTestsDir = gitRoot / "tests" / "fuzzing"
fuzzingCorpusesDir = fuzzingTestsDir / "corpus"
@ -44,3 +44,4 @@ cli do (testname {.argument.}: string,
let testProgram = fuzzingTestsDir / &"ssz_decode_{testname}.nim"
exec &"""nim "{fuzzNims}" "{fuzzer}" "{testProgram}" "{corpusDir}" """

View File

@ -1,5 +1,5 @@
import
testutils/fuzzing, faststreams/inputs,
testutils/fuzzing, faststreams/inputs, serialization/testing/tracing,
../../beacon_chain/ssz,
../../beacon_chain/spec/[datatypes, crypto, digest, datatypes]
@ -20,6 +20,11 @@ template sszFuzzingTest*(T: type) =
let reEncoded = SSZ.encode(decoded)
if payload != reEncoded:
when hasSerializationTracing:
# Run deserialization again to produce a seriazation trace
# (this is useful for comparing with the initial deserialization)
discard SSZ.decode(reEncoded, T)
echo "Payload with len = ", payload.len
echo payload
echo "Re-encoided payload with len = ", reEncoded.len

View File

@ -223,10 +223,7 @@ proc sszCheck(baseDir, sszType, sszSubType: string) =
of "FixedTestStruct": checkBasic(FixedTestStruct, dir, expectedHash)
of "VarTestStruct": checkBasic(VarTestStruct, dir, expectedHash)
of "ComplexTestStruct": checkBasic(ComplexTestStruct, dir, expectedHash)
of "BitsStruct":
discard
# Compile-time issues
# checkBasic(BitsStruct, dir, expectedHash)
of "BitsStruct": checkBasic(BitsStruct, dir, expectedHash)
else:
raise newException(ValueError, "unknown container in test: " & sszSubType)
else:

@ -1 +1 @@
Subproject commit 5d7cad792ff782672cc46a84cfcae341b55e2437
Subproject commit 81c24860e2622a15e05c81d15e3d1cc02c460870

@ -1 +1 @@
Subproject commit 16e6bcd16d71eba932978ebc22908da414f52db7
Subproject commit 501f94ad61ae22197d565ac39a00783a6e6f6faa

@ -1 +1 @@
Subproject commit 6382fdf88aa2e2179419348eb5771ba66a4a77e3
Subproject commit fcc5aa3a532cf42f9f1074f29df36fd69b3d2920