bitwarden / mobile

Retired Bitwarden mobile app for iOS and Android (MAUI/Xamarin).
https://bitwarden.com
GNU General Public License v3.0
152 stars 24 forks source link

FIDO2 Web Authn on Android displays a wrong text in Japanese #3345

Open yugui opened 4 months ago

yugui commented 4 months ago

Production Build

Steps To Reproduce

  1. When you have FIDO2 WebAuthn configured as the 2FA in your account and your display language is Japanese, try to login to your account with the Android App.
  2. Enter your login email account and press "Continue" (or "続行" in Japanese locale)
  3. Enter your master password and press "Log in with master password" (or "マスターパスワードによるログイン" in Japanese locale)
    • NOTE: This step assumes that the account has a login by the master password
  4. Now you are redirected to the Web Authn mobile connector page (https://vault.bitwarden.com/webauthn-mobile-connector.html) with a certain set of query parameters

Expected Result

The Web Authn mobile connector has a blue button whose display text is "Web Authn の認証".

The display text should be consistent with the string "Fido2AuthenticateWebAuthn" in the locale file src/Core/Resources/Localization/AppResources.ja.resx.

Actual Result

The Web Authn mobile connector has a blue button whose display text is "WebAuthn の認証".

The actual byte sequence of the display text is:

0x57, 0x65, 0x62, 0x41, 0x75, 0x74, 0x68, 0x6e,
0x20, 0xc3, 0xa3, 0xc2, 0x81, 0xc2, 0xae, 0xc3,
0xa8, 0xc2, 0xaa, 0xc2, 0x8d, 0xc3, 0xa8, 0xc2,
0xa8, 0xc2, 0xbc

Screenshots or Videos

Expected

Screenshot of the expected result (the display text of the button was modified by the inspection tool in Chrome): スクリーンショット 2024-07-07 11 34 43

Actual

Screenshot of the actual result: スクリーンショット 2024-07-07 11 34 50

Ref

FYI: Screenshot of the equivalent page in the Bitwarden Chrome Extension (webauthn-fallback-connector.html). スクリーンショット 2024-07-07 11 33 32

This is consistent with the expected result.

Additional Context

I believe you can reproduce an equivalent result by inserting a unicode emoji into the Fido2AuthenticateWebAuthn entry in the message localization file in an arbitrary locale.

If I understand correctly, the root cause of the issue is that the escape of multibyte characters is inconsistently implemented to the unescaping in the webauthn connector, and the mobile application passes a wrong data query parameter to https://vault.bitwarden.com/webauthn-mobile-connector.html.

  1. Bit.App.Pages.TwoFactorPageViewModel.Fido2AuthenticateAsync method generates the data parameter by passing the values to Bit.App.Utilities.AppHelpers.EncodeDataParameter.
  2. EncodeDataParameter first encodes the given object into JSON (JsonConvert.SerializeObject)), and then escape it with URL encoding (Uri.EscapeDataString)
  3. The method then replaces the escaped bytes in "%xx" notations back to a character by Convert.ToChar(Convert.ToUInt32($"0x{match.Groups[1].Value}", 16)).ToString(), and then the generated single byte string into a byte sequence with Encoding.UTF8.GetBytes.
  4. Finally, the method applies Base64

Operating System

Android

Operating System Version

14

Device

Pixel6

Build Version

2024.6.0 (10746)

yugui commented 4 months ago

Minimal Reproducible Example

This is the minimal reproducible example:

Encoder.cs:

using System;
using System.Text;
using System.Text.RegularExpressions;

class Encoder {
  static void Main(string[] args) {
    // c.f. https://github.com/bitwarden/mobile/blob/8a43bb465515d3a775e60eb083cf88ef0e3e70c8/src/Core/Pages/Accounts/TwoFactorPageViewModel.cs#L343
    // c.f. https://github.com/bitwarden/mobile/blob/8a43bb465515d3a775e60eb083cf88ef0e3e70c8/src/Core/Resources/Localization/AppResources.ja.resx#L2128
    var message = "WebAuthn の認証";
    var json = $"{{\"btnText\": \"{message}\"}}";

    // c.f. https://github.com/bitwarden/mobile/blob/main/src/Core/Utilities/AppHelpers.cs#L523
    string EncodeMultibyte(Match match)
     {
      return Convert.ToChar(Convert.ToUInt32($"0x{match.Groups[1].Value}", 16)).ToString();
     }

    var escaped = Uri.EscapeDataString(json);
    Console.WriteLine(escaped);
    var multiByteEscaped = Regex.Replace(escaped, "%([0-9A-F]{2})", EncodeMultibyte);
    var data = Convert.ToBase64String(Encoding.UTF8.GetBytes(multiByteEscaped));

    Console.WriteLine(data);
  }
}

Decoder.js:

// c.f. https://github.com/bitwarden/clients/blob/48de33fc7a360e0a23df3351b3ad63d1c5579aac/apps/web/src/connectors/common.ts#L18
function b64Decode(str) {
  return decodeURIComponent(
    Array.prototype.map
      .call(atob(str), (c) => {
        return "%" + ("00" + c.charCodeAt(0).toString(16)).slice(-2);
      })
     .join(""),
  );
}

// c.f. https://github.com/bitwarden/clients/blob/48de33fc7a360e0a23df3351b3ad63d1c5579aac/apps/web/src/connectors/webauthn.ts#L88
function decode(data) {
  const dataObj = JSON.parse(b64Decode(data));
  console.log(dataObj.btnText);
}

decode(process.argv[2]);

The next command would generate the same mojibake text as the one that WebAuth Mobile Connector displayed:

$ node Decoder.js $(mono Encoder.exe)
WebAuthn の認è

Fix

The next version of the Encoder would fix the issue, and make the message consumer correctly recover the original message value.

using System;
using System.Text;
using System.Text.RegularExpressions;

class Encoder {
  static void Main(string[] args) {
    // c.f. https://github.com/bitwarden/mobile/blob/8a43bb465515d3a775e60eb083cf88ef0e3e70c8/src/Core/Pages/Accounts/TwoFactorPageViewModel.cs#L343
    // c.f. https://github.com/bitwarden/mobile/blob/8a43bb465515d3a775e60eb083cf88ef0e3e70c8/src/Core/Resources/Localization/AppResources.ja.resx#L2128
    var message = "WebAuthn の認証";
    var json = $"{{\"btnText\": \"{message}\"}}";
    var data = Convert.ToBase64String(Encoding.UTF8.GetBytes(json));

    Console.WriteLine(data);
  }
}
$ node Decoder.js $(mono Encoder.exe)
WebAuthn の認証

Analysis

Let's take a look at the first multibyte character in the original message, "の" (U+306E), for example.

  1. Encoding into JSON preserves the character because ECMA-404 does not require character escapes in strings values except for " (double quotation) or \ (backslash).
  2. Uri.EscapeDataString internally calls System.UriHelper.EscapeStringToBuilder and it encodes the character into its UTF-8 byte sequence representation, and then individual bytes in the sequence are encoded into the form of "%xx".
    • U+306E is represented as 0xE3, 0x81, 0xAE in UTF-8
    • It is encoded into "%E3%81%AE"
  3. The invocation of Regex.Replace replaces the individual occurrences of "%xx" form with EncodeMultibyte
    • Convert.ToUInt32 in EncodeMultibyte converts the regex captures of "E3", "81", and "AE" into 0xE3u, 0x81u, and 0xAEu, respectively
    • Convert.ToChar converts individual integer values into their corresponding unicode code points. 0xE3u, 0x81u, and 0xAEu are converted into the following characters, respectively:
      • ã (U+00E3, LATIN SMALL LETTER A WITH TILDE)
      • a non-printable control character (U+0081)
      • ® (U+00AE, REGISTERED SIGN)
  4. Encoding.UTF8.GetBytes returns the UTF-8 byte sequence representation of the given string.
    • The characters ã, , and ® are converted into their corresponding byte sequences as the followings:
      • ã (U+00E3) --> 0xC3, 0xA3
      • U+0081 --> 0xC2, 0x81
      • ® (U+00AE) --> 0xC2", 0xAE
  5. Convert.ToBase64String converts the 6 bytes into "w6PCgcKu".

The consumer-side implementation reverses just the step (5), (4), and (1). Therefore, we get の (U+00E3, U+0031, U+00AE) instead of "の" (U306E).

Justification of the proposed fix

If I understand correctly, the intention of this conversion in AppHelpers::EncodeDataParameter is to keep the multibyte characters url-safe.

Therefore, just WebUtility.UrlEncode would be sufficient. It internally converts the given string into its UTF-8 byte sequence representation, and individual bytes into a url-safe characters.

In practice, Base64 encoding is also necessary for consistency with the consumer-side.

In any way, the earlier steps of (2), and (3) are not necessary at all. Both of WebUtility.UrlEncode and Convert.ToBase64String(Encoding.UTF8.GetBytes(str)) are safe to multibyte characters, and they are what the consumer-side implementation reverses.

daniellbw commented 4 months ago

Hi there,

This issue has been escalated for further investigation. If you have more information that can help us, please add it below.

Thanks!

yugui commented 4 months ago

Thank you for your response, @daniellbw.

Just in case, I have already prepared a fix for this issue in #3346