Runtime: Convert.FromBase64String different behavior in .NET Core 2.1 vs 2.0

[CIPop]

The following code does not throw in .NET Core 2.0:

```C#
string input = "randonendorsementkeyfortest==";
Convert.FromBase64String(input);

In .NET Core 2.1 this will throw:

Unhandled Exception: System.FormatException: The input is not a valid Base-64 string as it contains a non-base 64 character, more than two padding characters, or an illegal character among the padding characters.
at System.Convert.FromBase64CharPtr(Char* inputPtr, Int32 inputLength)
at System.Convert.FromBase64String(String s)
```

It's very likely that the exception is correctly identifying a malformed base64 string that we were using to test (I've verified that randonendorsementkeyfortest= - only one padding (=) character - is accepted by the decoder in 2.1).

I think it's worth noting the difference in the API Compat list.
Due to another bug, this caused our tests to fail after upgrading our Docker container to .NET Core 2.1 runtime for a __vstest__ project (TFM remained the same).

[danmosemsft]

@AtsushiKan looks like you made the last significant changes to base 64 decode in https://github.com/dotnet/coreclr/commit/e0fa11308de58684cde6bf8b7d0da287b48f01e9 so perhaps you can speak to this one.

I guess strictly this input should have only 1 padding? If the old implementation was tolerant of extra padding, it seems to me that we should do: it does not introduce any ambiguity, and avoids breaking code. If that's the case, this might be a servicing candidate?

[ghost]

I'll look at this tomorrow.

[ghost]

Stepping through the older implementation:

• The behavior is clearly the result of a subtle logic error, not an intended behavior. An extra "=" at the end of a empty string, for example, throws the correct exception.

• The extra "=" is not just ignored. It causes the api to return a garbage byte array. It decodes the last complete subgroup as if it were "bc==" rather than "abc=". In other words, it's the _fifth from last_ character that's discarded, not the illicit "=".

Unless we see widespread compat problems, I don't think it's worth replicating this quirk.

[stephentoub]

Unless we see widespread compat problems, I don't think it's worth replicating this quirk.

+1

The behavior is clearly the result of a subtle logic error, not an intended behavior.

Can we make sure we have tests for these various bugs?

[danmosemsft]

Thanks for investigating @AtsushiKan . @CIPop since you mention this was a test string, and the result produced in the old behavior is garbage, I don't think we will want to change this. Appreciate the report though.

[tracker1]

Any chance of seeing a fix prior to 3.0? I've worked around it for now, but it's annoying. I'm seeing this bug while trying to decode authentication tokens... (current workaround below)

    private static byte[] ConvertFromBase64String(string input) {
        string working = input.Replace('-', '+').Replace('_', '/');
        while (working.Length % 3 != 0) {
            working += '=';
        }
        try {
            return Convert.FromBase64String(working);
        } catch(Exception) {
            // .Net core 2.1 bug
            // https://github.com/dotnet/corefx/issues/30793
            try {
                return Convert.FromBase64String(input.Replace('-', '+').Replace('_', '/'));
            } catch(Exception) {}
            try {
                return Convert.FromBase64String(input.Replace('-', '+').Replace('_', '/') + "=");
            } catch(Exception) {}
            try {
                return Convert.FromBase64String(input.Replace('-', '+').Replace('_', '/') + "==");
            } catch(Exception) {}

            return null;
        }
    }

[gfoidl]

@tracker1

decode authentication tokens

these are base64url encoded, where + is replaced with - and / is replaced with _, and no padding = is added.
Hence in this case it isn't a bug, because it's a "different" encoding (while still based on base64).

For _base64url_ you can use WebEncoders (and hopefully sometime https://github.com/aspnet/Common/pull/338), which handles these encoding w/o need for manually fixing up things.

[tracker1]

@gfoidl It IS the same bug, because the translated string is valid, padded, base64...

Convert.FromBase64String("eyJqdGkiOiI1NjI3N2MzMi03YjQxLTRlYmItODY1Yy0yOWUyMTNkOWQ0OTAiLCJpc3MiOiJodHRwczovL2RldmF1dGgtdm9jZW0vIiwiYXVkIjoiaHR0cDovL2xvY2FsaG9zdDo4MDgwLyIsInN1YiI6ImNzaXBlcyIsImVtbCI6ImNzaXBlc0BydW5iZWNrLm5ldCIsImZubSI6IkNocmlzIiwibG5tIjoiU2lwZXMiLCJhZmYiOlsiWFgiXSwicm9sIjpbInJicHIiLCJyYnNyIiwicmJscHIiLCJyYmxzciIsInJicXByIiwicmJxc3IiLCJyYnN1IiwicmJjYSIsInJic2EiXSwiaWF0IjoxNTM3ODk1NjA3LCJleHAiOjE1Mzc5Mzg4MDd9==")

Blows up... it's valid, padded, base64 ...

[gfoidl]

Blows up... it's valid, padded, base64 ...

Yeah, it has to blow up when it follows the rules.

A valid base64-encoded string length is always a multiple of 4. (If there is not enough input-data, padding = will therefore get added).

Your given string has length 402, so 402 % 4 = 2. Hence it is not a valid base64 string.
Try to decode without the final padding (so the string-length = 400, and 400 % 4 = 0), it won't blow up and give you the expected byte[300] result.

BTW: your workaround is really hacky :wink:

[tracker1]

@gfoidl My bad... I remembered incorrectly.. for some reason was thinking 3 encoded characters for every 2 bytes, not 4 encoded per 3 bytes. You are correct. :-)

and yeah, it was incredibly hacky... I should have looked back at the spec instead of working from memory. I found this bug, and thought it applied.

my final wrapper in case anyone needs similar, though it would be nice if the built in FromBase64String accounted for base64url strings too.

private static byte[] ConvertFromBase64String(string input) {
    if (String.IsNullOrWhiteSpace(input)) return null;
    try {
        string working = input.Replace('-', '+').Replace('_', '/'); ;
        while (working.Length % 4 != 0) {
            working += '=';
        }
        return Convert.FromBase64String(working);
    } catch(Exception) {
        return null;
    }
}

[ArkarMoe93]

I'm using .Net Core 2.2. I faced the same issue.
But I added one new Line at Decrypt() and it works for me. Thanks
`
public string Decrypt(string value)
{
if (string.IsNullOrEmpty(value)) return value;
try
{
value = value.Replace(" ", "+");
var fullCipher = Convert.FromBase64String(value);

            var iv = new byte[16];
            var cipher = new byte[fullCipher.Length - iv.Length];

            Buffer.BlockCopy(fullCipher, 0, iv, 0, iv.Length);
            Buffer.BlockCopy(fullCipher, iv.Length, cipher, 0, fullCipher.Length - iv.Length);
            var key = Encoding.UTF8.GetBytes(KeyValue);

            using (var aesAlg = Aes.Create())
            {
                using (var decryptor = aesAlg.CreateDecryptor(key, iv))
                {
                    string result;
                    using (var msDecrypt = new MemoryStream(cipher))
                    {
                        using (var csDecrypt = new CryptoStream(msDecrypt, decryptor, CryptoStreamMode.Read))
                        {
                            using (var srDecrypt = new StreamReader(csDecrypt))
                            {
                                result = srDecrypt.ReadToEnd();
                            }
                        }
                    }

                    return result;
                }
            }
        }
        catch (Exception ex)
        {
            return string.Empty;
        }
    }`

[mithunta]

Looks like issue is not fixed in 3.0 as well. Do we know when this issue will be fixed?

[gfoidl]

@mithunta just to make sure you're not mixing _base64_ with _base64url_ as they are similar, but different encodings (see above).

What issue do you want to have fixed?
For _base64url_ you can use something like https://www.nuget.org/packages/gfoidl.Base64/ which supports that.

[mithunta]

@gfoidl - I was using built in System function - Convert.FromBase64String(data). I used the suggested solution from the post by @tracker1 by replacing the literals. I did not try this base64url package.