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

Ensure the hash length (2x) #897

Merged
merged 3 commits into from
Jul 11, 2019

Conversation

shargon
Copy link
Member

@shargon shargon commented Jul 8, 2019

No description provided.

@@ -44,7 +43,10 @@ protected UIntBase(int bytes, byte[] value)
/// </summary>
void ISerializable.Deserialize(BinaryReader reader)
{
reader.Read(data_bytes, 0, data_bytes.Length);
if (reader.Read(data_bytes, 0, data_bytes.Length) != data_bytes.Length)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have a SafeRead method somewhere, to ensure this.

Copy link
Contributor

@lock9 lock9 Jul 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igormcoelho I searched for it in code ("safe" and "reader.Read").
I found similar code in ECPoint class, but it has a length check in a second step.

public static ECPoint DeserializeFrom(BinaryReader reader, ECCurve curve)
        {
            int expectedLength = (curve.Q.GetBitLength() + 7) / 8;
            byte[] buffer = new byte[1 + expectedLength * 2];
            buffer[0] = reader.ReadByte();
            switch (buffer[0])
            {
                case 0x02:
                case 0x03:
                    reader.Read(buffer, 1, expectedLength);
                    return DecodePoint(buffer.Take(1 + expectedLength).ToArray(), curve);
                case 0x04:
                    reader.Read(buffer, 1, expectedLength * 2);
                    return DecodePoint(buffer, curve);
                default:
                    throw new FormatException("Invalid point encoding " + buffer[0]);
            }
        }
public static ECPoint DecodePoint(byte[] encoded, ECCurve curve)
        {
            ECPoint p = null;
            int expectedLength = (curve.Q.GetBitLength() + 7) / 8;
            switch (encoded[0])
            {
                case 0x02: // compressed
                case 0x03: // compressed
                    {
                        if (encoded.Length != (expectedLength + 1))
                            throw new FormatException("Incorrect length for compressed encoding");
                        int yTilde = encoded[0] & 1;
                        BigInteger X1 = new BigInteger(encoded.Skip(1).Reverse().Concat(new byte[1]).ToArray());
                        p = DecompressPoint(yTilde, X1, curve);
                        break;
                    }
                case 0x04: // uncompressed
                    {
                        if (encoded.Length != (2 * expectedLength + 1))
                            throw new FormatException("Incorrect length for uncompressed/hybrid encoding");
                        BigInteger X1 = new BigInteger(encoded.Skip(1).Take(expectedLength).Reverse().Concat(new byte[1]).ToArray());
                        BigInteger Y1 = new BigInteger(encoded.Skip(1 + expectedLength).Reverse().Concat(new byte[1]).ToArray());
                        p = new ECPoint(new ECFieldElement(X1, curve), new ECFieldElement(Y1, curve), curve);
                        break;
                    }
                default:
                    throw new FormatException("Invalid point encoding " + encoded[0]);
            }
            return p;
        }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will fix it

lock9
lock9 previously approved these changes Jul 9, 2019
Copy link
Contributor

@lock9 lock9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to check if this fix is also required in 3x

Copy link
Member Author

@shargon shargon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is done @lock9, ready to be merged

@shargon shargon merged commit 425eaa7 into neo-project:master-2.x Jul 11, 2019
@shargon shargon deleted the fix-unit-deserialize branch July 11, 2019 08:09
Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
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

Successfully merging this pull request may close these issues.

3 participants