Cookie.Expiry ArgumentOutOfRangeException crash caused by conversion of extremely large date time

[mbragg12]

Bug Report

• What version of the product are you using? 106

• What architecture x86 or x64? x86

• What version of .Net? .Net 4.x

• On what operating system? Any, debugging on Win10

• What steps will reproduce the problem?

  • Use something like DB Browser for SQLite to modify the expiry date of a cookie to 759797148870000000
  • Perform an action that would cause the cookie to be visited

• What is the expected output? What do you see instead?

  • no crash

• Does this problem also occur in the CEF Sample Application

  • No

• Please provide any additional information below.

  • We have been seeing crashes from users with older cookie stores. The issue is related to the changes in the conversion from the CEF time format changed in 106.
  • There are several sites that appear to have set extremely large expiry dates in the past that are causing the issue. Pixabay is a common one we're seeing. 759797148870000000 is the exact value that it was setting.
  • In the call to FromBaseTimeToDateTime, the conversion of return DateTime.FromFileTime(val * 10); causes an exception to be thrown. The exception is System.ArgumentOutOfRangeException Not a valid Win32 FileTime.
  • FileTime's max value is 2650467743999999999. See FromFileTimeUtc in https://referencesource.microsoft.com/#mscorlib/system/datetime.cs,63cfb478622b68fa for the gory details on how that number is calculated.
  • Stack Trace: (I was testing if it could affect other values, thus SetLastAccessDate, but logic the same)

    
    at System.DateTime.FromFileTime(Int64 fileTime)
    at CefSharp.Internals.CefTimeUtils.FromBaseTimeToDateTime(Int64 val) in C:\Source\lowkey\cef106\CefSharpBuild\CefSharp\CefSharp\Internals\CefTimeUtils.cs:line 44
    at CefSharp.Cookie.SetLastAccessDate(Int64 baseTime) in C:\Source\lowkey\cef106\CefSharpBuild\CefSharp\CefSharp\Cookie.cs:line 103
    at CefSharp.Internals.TypeConversion.FromNative(CefStructBase<CefCookieTraits>* cefCookie) in C:\Source\lowkey\cef106\CefSharpBuild\CefSharp\CefSharp.Core.Runtime\Internals\TypeConversion.h:line 260
    at CefSharp.Internals.CefCookieVisitorAdapter.Visit(CefCookieVisitorAdapter* , CefStructBase<CefCookieTraits>* cefCookie, Int32 count, Int32 total, Boolean* deleteCookie) in C:\Source\lowkey\cef106\CefSharpBuild\CefSharp\CefSharp.Core.Runtime\Internals\CefCookieVisitorAdapter.h:line 35

  This exception was originally thrown at this call stack: [External Code] CefSharp.Internals.CefTimeUtils.FromBaseTimeToDateTime(long) in CefTimeUtils.cs CefSharp.Cookie.SetLastAccessDate(long) in Cookie.cs CefSharp::Internals::TypeConversion::FromNative(CefStructBase&) in TypeConversion.h CefSharp::Internals::CefCookieVisitorAdapter::Visit(CefStructBase&, int, int, bool&) in CefCookieVisitorAdapter.h

[mbragg12]

@amaitland I can look at making a PR for this issue if desired. Just need to know if you would rather sanitize the values in static Cookie FromNative or put a max value check in FromBaseTimeToDateTime.

[amaitland]

759797148870000000 is the exact value that it was setting.

Do you happen to know what date this converts to?

put a max value check in FromBaseTimeToDateTime.

Let's go with this option, thanks 👍

[mbragg12]

Do you happen to know what date this converts to?

No clue. It doesn't seem like a valid date but we have seen it in a few DB's from a couple sites now. So Chromium thought it was valid for something at some point.

put a max value check in FromBaseTimeToDateTime.

Let's go with this option, thanks 👍

This is the check we are using as a fix tentatively. Waiting on feedback, to make sure. Look good to you?

         public static DateTime FromBaseTimeToDateTime(long val)
         {
            //FileTime max value is 2650467743999999999
            if (val > 265046774399999999)
                val = 265046774399999999;

             return DateTime.FromFileTime(val * 10);
         }

[amaitland]

Thanks, I'd like to use MaxValue if at all possible, make for easier comparison.

See what you think of https://github.com/cefsharp/CefSharp/commit/9dc158693d6aa25cc6905ea10db90f1f548857e5

Unit tests will need to be added still.

[mbragg12]

I like yours better. Much cleaner.

[amaitland]

This should be fixed in 106.0.290.

Adding a custom conversion implementation should also be possible see https://github.com/cefsharp/CefSharp/commit/710508b0ff592c0de6266501c23bf839310a5fdd#diff-4f6c2a768c993bf3274c45a7cfeda5bdea738e11130e6dc01daec795a6a07931R25