moov-io / iso8583

A golang implementation to marshal and unmarshal iso8583 message.
https://moov.io
Apache License 2.0
353 stars 105 forks source link

field unpack based on ASCII Encoder panic: runtime error: slice bounds out of range #217

Closed jerome-laforge closed 1 year ago

jerome-laforge commented 1 year ago

while fuzzing, the fuzzer reports me:

panic: runtime error: slice bounds out of range [:-1] [recovered]
    panic: runtime error: slice bounds out of range [:-1]

github.com/moov-io/iso8583/encoding.asciiEncoder.Decode({}, {0xc0001f2a2e, 0x0, 0x12}, 0xffffffffffffffff)
    /home/user/dev/opensource/iso8583/encoding/ascii.go:30 +0x7ea
github.com/moov-io/iso8583/field.(*Numeric).Unpack(0xc000012090, {0xc0001f2a2c, 0x2, 0x14})
    /home/user/dev/opensource/iso8583/field/numeric.go:115 +0x4ee
github.com/moov-io/iso8583.(*Message).Unpack(0xc000296760, {0xc0001f2a20, 0xe, 0x20})
    /home/user/dev/opensource/iso8583/message.go:196 +0xa0d

Because, driver has to check the length is not negative.

With the fix below, fuzzer is happier.

diff --git a/prefix/ascii.go b/prefix/ascii.got.go
index d45418a..590ba6c 100644
--- a/prefix/ascii.go
+++ b/prefix/ascii.go
@@ -42,6 +42,10 @@ func (p *asciiVarPrefixer) DecodeLength(maxLen int, data []byte) (int, int, erro
                return 0, 0, err
        }

+       if dataLen < 0 {
+               return 0, 0, fmt.Errorf("data length: %d is negative", dataLen)
+       }
+
        if dataLen > maxLen {
                return 0, 0, fmt.Errorf("data length: %d is larger than maximum %d", dataLen, maxLen)
        }
alovak commented 1 year ago

closed by #218

alovak commented 1 year ago

@jerome-laforge thanks for reporting the issue!

jerome-laforge commented 1 year ago

I don't know if we need to open an issue on vulndb:

https://github.com/golang/vulndb/issues/new?assignees=&labels=Needs+Triage%2CDirect+External+Report&template=new_third_party_vuln.yml&title=x%2Fvulndb%3A+potential+Go+vuln+in+%3Cpackage%3E