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

CVE-2021-27293: Fix NewDateRegex #1556

Closed
b-c-ds opened this issue Feb 16, 2021 · 13 comments · Fixed by #1557
Closed

CVE-2021-27293: Fix NewDateRegex #1556

b-c-ds opened this issue Feb 16, 2021 · 13 comments · Fixed by #1557

Comments

@b-c-ds
Copy link
Contributor

b-c-ds commented Feb 16, 2021

This advisory was emailed to the maintainer. Posting here as an issue as requested.

Doyensec Vulnerability Advisory

  • Regular Expression Denial of Service (REDoS) in RestSharp
  • Affected Product: RestSharp (All released versions)
  • Vendor: https://restsharp.dev
  • Severity: Medium
  • Vulnerability Class: Denial of Service
  • Status: Open
  • Author: Ben Caller (Doyensec)

SUMMARY

The .NET library RestSharp uses a regular expression which is vulnerable to Regular Expression Denial of Service (REDoS) when converting strings into DateTimes. If a server responds with a malicious string, the client using RestSharp will be stuck processing it for an exceedingly long time. This allows the remote server to trigger a Denial of Service.

TECHNICAL DESCRIPTION

The vulnerable regular expression is NewDateRegex in RestSharp.Extensions.StringExtensions:

static readonly Regex NewDateRegex = new Regex(@"newDate\((-?\d+)*\)");

It is used by the ParseJsonDate function when deserializing JSON responses into classes with DateTime properties.

Due to the (-?\d+)* part containing nested repeats, this regular expression has catastrophic backtracking when processing a long string of digits. The behaviour occurs as long as the digits are not followed immediately by a closing parenthesis ')'.

An example of a REDoS payload is new Date(12345678901234567890123456789012345.

The space between 'new' and 'Date' is required due to pre-processing in ParseJsonDate:

if (input.Contains("new Date("))
{
input = input.Replace(" ", "");

The complexity is exponential: increasing the length of the malicious string of digits by one makes processing take about twice as long. On my laptop, 27 digits takes about 16 seconds to process and 28 digits takes about 32 seconds, so a string with 54 digits should take approximately 68 years to process.

The vulnerable regular expression was first introduced in commit 373a0a3

REPRODUCTION STEPS

The REDoS can be triggered by calling RestSharp.Extensions.StringExtensions.ParseJsonDate directly, or by deserializing JSON responses into a class with a property of type DateTime.

Example C# code to see the effect of the REDoS is attached below. Changing the length of the string of zeroes will change the processing time.

using System;
using System.Globalization;
using RestSharp;
using RestSharp.Extensions;
using RestSharp.Serialization.Json;

namespace DoyensecRestSharpRedosTest
{
    class Thing
    {
        public DateTime Time { get; set; }
    }

    class Program
    {
        static void ByParseJsonDate()
        {
            var redos = "new Date(" + new String('0', 30);
            DateTime dt = StringExtensions.ParseJsonDate(redos, CultureInfo.InvariantCulture);
        }

        static void ByDeserializer()
        {
            var redosJson = @"{""Time"": ""new Date(" + new String('0', 30) + @"""}";
            var thing = (new JsonDeserializer()).Deserialize<Thing>(new RestResponse {Content = redosJson});
            Console.WriteLine(thing.Time);
        }

        static void ByRequest()
        {
            // Server should return JSON {"Time": "new Date(000000000000000000000000000000"}
            Thing thing = new RestClient("https://localhost/thing.json").Execute<Thing>(new RestRequest(Method.GET)).Data;
            Console.WriteLine(thing.Time);
        }

        static void Main(string[] args)
        {
            ByDeserializer(); // or try ByParseJsonDate() or ByRequest()
        }
    }
}

REMEDIATION

Fix NewDateRegex. We propose simply removing the asterisk:

    static readonly Regex NewDateRegex = new Regex(@"newDate\((-?\d+)\)");

.

Doyensec (www.doyensec.com) is an independent security research and development company focused on vulnerability discovery and remediation. We work at the intersection of software development and offensive engineering to help companies craft secure code.

Copyright 2021 by Doyensec LLC. All rights reserved.

Permission is hereby granted for the redistribution of this advisory, provided that it is not altered except by reformatting it, and that due credit is given. Permission is explicitly given for insertion in vulnerability databases and similar, provided
that due credit is given. The information in the advisory is believed to be accurate at the time of publishing based on
currently available information, and it is provided as-is, as a free service to the community by Doyensec LLC. There are
no warranties with regard to this information, and Doyensec LLC does not accept any liability for any direct, indirect, or
consequential loss or damage arising from use of, or reliance on, this information.

b-c-ds added a commit to b-c-ds/RestSharp that referenced this issue Feb 16, 2021
Previously it had exponential worst-case complexity and was vulnerable to REDoS.
@b-c-ds b-c-ds changed the title Fix NewDateRegex CVE-2021-27293: Fix NewDateRegex Mar 5, 2021
@b-c-ds
Copy link
Contributor Author

b-c-ds commented Mar 5, 2021

Assigned CVE-2021-27293

@abhijeet490
Copy link

abhijeet490 commented Apr 20, 2021

@bcaller I can see a fix to this vulnerability, but on which version can we get this update?

@b-c-ds
Copy link
Contributor Author

b-c-ds commented Apr 20, 2021

@abhijeet490 The patch has not reviewed or merged. If you want the fix anytime soon you may need to make a fork.

alexeyzimarev pushed a commit that referenced this issue May 5, 2021
* Fix NewDateRegex in StringExtensions #1556

Previously it had exponential worst-case complexity and was vulnerable to REDoS.

* Simple test for new Date(123)
@neil-gok
Copy link

neil-gok commented Jul 6, 2021

I see that this issue has been closed, but the vulnerability still exists in version 106.11.7. On what version will this fix be released?

@b-c-ds
Copy link
Contributor Author

b-c-ds commented Jul 9, 2021

106.11.8-alpha.0.13 😆

@RajRele
Copy link

RajRele commented Jul 13, 2021

When could we expect a stable release which includes this fix?

@gdoron
Copy link

gdoron commented Jul 15, 2021

I'm honestly asking, is it the common practice to share CVE publicly before the fix was released and adopted?

@gavinBurtonStoreFeeder
Copy link

gavinBurtonStoreFeeder commented Jul 15, 2021

I'm honestly asking, is it the common practice to share CVE publicly before the fix was released and adopted?

CVE's are already public knowledge. But its disappointing that its been since February without a patch.

@b-c-ds This issue should not be closed. Can you please confirm whether an official release is planned?

@b-c-ds
Copy link
Contributor Author

b-c-ds commented Jul 15, 2021

@gavinBurtonStoreFeeder I'm not a maintainer. It won't let me reopen the issue. Sorry. I only reported this issue and contributed one pull request. I don't think this project is super active so I wouldn't hold my breath waiting for a non-alpha release.
Also, if you aren't deserialising DateTimes you should be able to ignore this vulnerability.

@gavinBurtonStoreFeeder
Copy link

gavinBurtonStoreFeeder commented Jul 15, 2021

@gavinBurtonStoreFeeder I'm not a maintainer. It won't let me reopen the issue. Sorry. I only reported this issue and contributed one pull request. I don't think this project is super active so I wouldn't hold my breath waiting for a non-alpha release.
Also, if you aren't deserialising DateTimes you should be able to ignore this vulnerability.

Fair enough. Thanks for raising the GH issue. There is a fix underway but I feel like restsharp is abandonware now.

@Arslan-Ashfaq
Copy link

Arslan-Ashfaq commented Oct 26, 2021

This issue still exist when this will be fixed.

@alexeyzimarev
Copy link
Member

alexeyzimarev commented Oct 30, 2021

its been since February without a patch.

The PR was included to 106.12.0 release

restsharp is abandonware now.

It is a typical ".NET community project", isn't it? 99M downloads, zero sponsors, little contributions from non-maintainers, but lots of complaints (like the comment just above).

Big kudos to @b-c-ds for reporting and fixing the issue, it's now released, the issue is closed.

@gavinBurtonStoreFeeder
Copy link

gavinBurtonStoreFeeder commented Nov 1, 2021

its been since February without a patch.

The PR was included to 106.12.0 release

restsharp is abandonware now.

It is a typical ".NET community project", isn't it? 99M downloads, zero sponsors, little contributions from non-maintainers, but lots of complaints (like the comment just above).

Big kudos to @b-c-ds for reporting and fixing the issue, it's now released, the issue is closed.

You quoted me out of context by starting halfway through the sentence. And I made that comment in july, 5 months after the PR was created.

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 a pull request may close this issue.

8 participants