search

moov-io / iso8583

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

Return error when the lenght of the field value does not match the spec fixed lenght #179

Closed alovak closed 2 years ago

alovak commented 2 years ago

Currently, if we set an empty value to the field with the fixed and variable length prefix, there will be no error when we pack such a message. Receiving side will get malformed data and will try to parse it...

It looks like for empty values we skip the length check, but we should not:

https://github.com/moov-io/iso8583/blob/master/field/string.go#L67:

    if len(packed) == 0 {
        return []byte{}, nil
    }

this is not a simple fix, as this if/return was added to fix other issue: https://github.com/moov-io/iso8583/pull/149

Example of the error when field 48 - Acceptor name is empty and receiving party is trying to read LLL prefix (which was not put into the message as we skip adding prefix because of ^^) and reads the value of the next field (49, currency code) instead:

failed to unpack field 48 (Institution/Merchant Name): failed to decode length: data length: 840 is larger than maximum 25
mfdeveloper508 commented 2 years ago

Right, The code is very dangerous code main parser that included MTI(bitmap) don't run correctly it seems that we should update composite field only for https://github.com/moov-io/iso8583/pull/149

alovak commented 2 years ago

Hey @Dakinola892 as you were the author of #149 I want to ask for your feedback and maybe help us with finding the best solution to the issue.

Dakinola892 commented 2 years ago

I believe I added #149 as a band-aid fix to a problem we were having locally with parsing, but with the updatest to moov its no longer a problem, so i think mfdevelopers's PR to remove it is a decent enough solution