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

BigInteger.TryParse out-of-bounds access #28652

Closed
Metalnem opened this issue Feb 8, 2019 · 5 comments
Closed

BigInteger.TryParse out-of-bounds access #28652

Metalnem opened this issue Feb 8, 2019 · 5 comments
Assignees
Milestone

Comments

@Metalnem
Copy link

Metalnem commented Feb 8, 2019

BigInteger.TryParse overloads accepting ReadOnlySpan<char> are ignoring the length of the span, and are reading beyond its end if there are more digits available. Here's the program to reproduce this:

using System;
using System.Numerics;

namespace BigInt
{
  public class Program
  {
    public static void Main(string[] args)
    {
      var s = "123456789";
      var span = s.AsSpan(0, 1);

      if (BigInteger.TryParse(span, out var result))
      {
        Console.WriteLine(result);
      }
    }
  }
}

This program should be printing 1, but it's printing 123456789 instead. My running environment looks like this:

.NET Core SDK (reflecting any global.json):
 Version:   2.2.103
 Commit:    8edbc2570a

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  10.14
 OS Platform: Darwin
 RID:         osx.10.14-x64
 Base Path:   /usr/local/share/dotnet/sdk/2.2.103/

Found via SharpFuzz.

@benaadams
Copy link
Member

/cc @tannergooding, @ViktorHofer, @bartonjs

@tannergooding
Copy link
Member

Looks like the issue is here: https://source.dot.net/#System.Runtime.Numerics/System/Globalization/FormatProvider.Number.cs,564

TryStringToNumber doesn't pass in the length of str to ParseNumber, so it doesn't terminate at the right point.

@tannergooding
Copy link
Member

We are handling this correctly for Number.Parsing.cs, as we pass in the end-point as well: https://source.dot.net/#System.Private.CoreLib/shared/System/Number.Parsing.cs,1847

The ideal fix would likely be to no longer use pointers here and just use the ROSpan end-to-end (https://github.com/dotnet/coreclr/issues/18162). However, the short-term fix would be to just update the Number type used by BigInteger to do the same as the number type we have in corelib.

@tannergooding
Copy link
Member

CC. @danmosemsft

@stephentoub
Copy link
Member

We had the same issue for primitive parsing, fixed in dotnet/coreclr#17808. I didn't realize there was a copy of the code for BigInteger.

@stephentoub stephentoub self-assigned this Feb 8, 2019
@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 3.0 milestone Feb 1, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants