Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ID3Decoder's handling of APIC entries improperly assumes variable encoding for MIME type #72

Open
gsuberland opened this issue Oct 5, 2023 · 1 comment

Comments

@gsuberland
Copy link

The MIME type string is assumed to be of variable encoding, but it is not. The specification says its type is <text string>, not <text string according to encoding>, so it must always be ISO-8859-1. As such, GetASCIIString should be called instead of the dynamically selected getstr.

// get attributes
int position = 1;
string mime = getstr(Data, ref position, true);
byte type = Data[position++];
string description = getstr(Data, ref position, true);
// get image content
int datalength = Data.Length - position;
byte[] imgdata = new byte[datalength];
Array.Copy(Data, position, imgdata, 0, datalength);

This causes a crash with System.OverflowException when the entry contains no data because the position ends up improperly offset past the end of the string, resulting in position equalling Data.Length + 1, which makes datalength negative.

This can be fixed by swapping line 387 out for:

string mime = GetASCIIString(Data, ref position, true);

This fixes the issue and prevents the crash.

@gsuberland
Copy link
Author

As an aside, if you ever release a version for .NET 5 or later (rather than targeting .NET Standard), the Encoding.ASCII references should be swapped out for Encoding.Latin1, which guarantees the correct ISO-8859-1 codepage. It only really matters when trying to use extended characters (beyond the 7-bit character set) in ASCII, but the discrepancy can lead to erroneous decoding/encoding of tags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant