[SG-671] OTP Menu Screen causes Crash on Android (#2097)

* [SG-671] removed unnecessary calc of otpauth period. protected cal of otpauth from crashing the app if url has a wrong format.

* [SG-671] changed logger

* [SG-671] Refactored GetQueryParams code to used HttpUtility.ParseQueryString.

* [SG-671] refactor and null protection.

* [SG-671] code format

* [SG-671] fixed bug where totp circle countdown was fixed to 30.

* [SG-167] added fallback for uri check. Changed all default totp timers to constant.

* [SG-671] missed unsaved file

* [SG-671] simplified code
This commit is contained in:
André Bispo 2022-09-26 17:51:03 +01:00 committed by GitHub
parent 70ee24d82a
commit 2f4cd36595
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 25 additions and 29 deletions

View file

@ -40,8 +40,8 @@ namespace Bit.App.Pages
private string _totpCode; private string _totpCode;
private string _totpCodeFormatted; private string _totpCodeFormatted;
private string _totpSec; private string _totpSec;
private double _totpInterval = Constants.TotpDefaultTimer;
private bool _totpLow; private bool _totpLow;
private DateTime? _totpInterval = null;
private string _previousCipherId; private string _previousCipherId;
private byte[] _attachmentData; private byte[] _attachmentData;
private string _attachmentFilename; private string _attachmentFilename;
@ -241,7 +241,7 @@ namespace Bit.App.Pages
Page.Resources["textTotp"] = ThemeManager.Resources()[value ? "text-danger" : "text-default"]; Page.Resources["textTotp"] = ThemeManager.Resources()[value ? "text-danger" : "text-default"];
} }
} }
public double TotpProgress => string.IsNullOrEmpty(TotpSec) ? 0 : double.Parse(TotpSec) * 100 / 30; public double TotpProgress => string.IsNullOrEmpty(TotpSec) ? 0 : double.Parse(TotpSec) * 100 / _totpInterval;
public bool IsDeleted => Cipher.IsDeleted; public bool IsDeleted => Cipher.IsDeleted;
public bool CanEdit => !Cipher.IsDeleted; public bool CanEdit => !Cipher.IsDeleted;
@ -265,6 +265,7 @@ namespace Bit.App.Pages
{ {
_totpTickHelper = new TotpHelper(Cipher); _totpTickHelper = new TotpHelper(Cipher);
_totpTickCancellationToken?.Cancel(); _totpTickCancellationToken?.Cancel();
_totpInterval = _totpTickHelper.Interval;
_totpTickCancellationToken = new CancellationTokenSource(); _totpTickCancellationToken = new CancellationTokenSource();
_totpTickTask = new TimerTask(_logger, StartCiphersTotpTick, _totpTickCancellationToken).RunPeriodic(); _totpTickTask = new TimerTask(_logger, StartCiphersTotpTick, _totpTickCancellationToken).RunPeriodic();
} }
@ -284,6 +285,7 @@ namespace Bit.App.Pages
await _totpTickHelper.GenerateNewTotpValues(); await _totpTickHelper.GenerateNewTotpValues();
TotpSec = _totpTickHelper.TotpSec; TotpSec = _totpTickHelper.TotpSec;
TotpCodeFormatted = _totpTickHelper.TotpCodeFormatted; TotpCodeFormatted = _totpTickHelper.TotpCodeFormatted;
_totpInterval = _totpTickHelper.Interval;
} }
catch (Exception ex) catch (Exception ex)
{ {
@ -429,7 +431,6 @@ namespace Bit.App.Pages
{ {
if (Cipher == null || Cipher.Type != Core.Enums.CipherType.Login || Cipher.Login.Totp == null) if (Cipher == null || Cipher.Type != Core.Enums.CipherType.Login || Cipher.Login.Totp == null)
{ {
_totpInterval = null;
return; return;
} }
_totpCode = await _totpService.GetCodeAsync(Cipher.Login.Totp); _totpCode = await _totpService.GetCodeAsync(Cipher.Login.Totp);
@ -449,7 +450,6 @@ namespace Bit.App.Pages
else else
{ {
TotpCodeFormatted = null; TotpCodeFormatted = null;
_totpInterval = null;
} }
} }

View file

@ -22,7 +22,6 @@ namespace Bit.App.Pages
private bool _websiteIconsEnabled; private bool _websiteIconsEnabled;
private string _iconImageSource = string.Empty; private string _iconImageSource = string.Empty;
public int interval { get; set; }
private double _progress; private double _progress;
private string _totpSec; private string _totpSec;
private string _totpCodeFormatted; private string _totpCodeFormatted;
@ -37,7 +36,6 @@ namespace Bit.App.Pages
Cipher = cipherView; Cipher = cipherView;
WebsiteIconsEnabled = websiteIconsEnabled; WebsiteIconsEnabled = websiteIconsEnabled;
interval = _totpService.GetTimeInterval(Cipher.Login.Totp);
CopyCommand = new AsyncCommand(CopyToClipboardAsync, CopyCommand = new AsyncCommand(CopyToClipboardAsync,
onException: ex => _logger.Value.Exception(ex), onException: ex => _logger.Value.Exception(ex),
allowsMultipleExecutions: false); allowsMultipleExecutions: false);

View file

@ -1,5 +1,6 @@
using System; using System;
using System.Threading.Tasks; using System.Threading.Tasks;
using Bit.Core;
using Bit.Core.Abstractions; using Bit.Core.Abstractions;
using Bit.Core.Models.View; using Bit.Core.Models.View;
using Bit.Core.Utilities; using Bit.Core.Utilities;
@ -10,26 +11,26 @@ namespace Bit.App.Utilities
{ {
private ITotpService _totpService; private ITotpService _totpService;
private CipherView _cipher; private CipherView _cipher;
private int _interval;
public TotpHelper(CipherView cipher) public TotpHelper(CipherView cipher)
{ {
_totpService = ServiceContainer.Resolve<ITotpService>("totpService"); _totpService = ServiceContainer.Resolve<ITotpService>("totpService");
_cipher = cipher; _cipher = cipher;
_interval = _totpService.GetTimeInterval(cipher?.Login?.Totp); Interval = _totpService.GetTimeInterval(cipher?.Login?.Totp);
} }
public string TotpSec { get; private set; } public string TotpSec { get; private set; }
public string TotpCodeFormatted { get; private set; } public string TotpCodeFormatted { get; private set; }
public double Progress { get; private set; } public double Progress { get; private set; }
public double Interval { get; private set; } = Constants.TotpDefaultTimer;
public async Task GenerateNewTotpValues() public async Task GenerateNewTotpValues()
{ {
var epoc = CoreHelpers.EpocUtcNow() / 1000; var epoc = CoreHelpers.EpocUtcNow() / 1000;
var mod = epoc % _interval; var mod = epoc % Interval;
var totpSec = _interval - mod; var totpSec = Interval - mod;
TotpSec = totpSec.ToString(); TotpSec = totpSec.ToString();
Progress = totpSec * 100 / 30; Progress = totpSec * 100 / Interval;
if (mod == 0 || string.IsNullOrEmpty(TotpCodeFormatted)) if (mod == 0 || string.IsNullOrEmpty(TotpCodeFormatted))
{ {
TotpCodeFormatted = await TotpUpdateCodeAsync(); TotpCodeFormatted = await TotpUpdateCodeAsync();

View file

@ -33,6 +33,7 @@
public const int SelectFileRequestCode = 42; public const int SelectFileRequestCode = 42;
public const int SelectFilePermissionRequestCode = 43; public const int SelectFilePermissionRequestCode = 43;
public const int SaveFileRequestCode = 44; public const int SaveFileRequestCode = 44;
public const int TotpDefaultTimer = 30;
public static readonly string[] AndroidAllClearCipherCacheKeys = public static readonly string[] AndroidAllClearCipherCacheKeys =
{ {

View file

@ -24,7 +24,7 @@ namespace Bit.Core.Services
{ {
return null; return null;
} }
var period = 30; var period = Constants.TotpDefaultTimer;
var alg = CryptoHashAlgorithm.Sha1; var alg = CryptoHashAlgorithm.Sha1;
var digits = 6; var digits = 6;
var keyB32 = key; var keyB32 = key;
@ -117,7 +117,7 @@ namespace Bit.Core.Services
public int GetTimeInterval(string key) public int GetTimeInterval(string key)
{ {
var period = 30; var period = Constants.TotpDefaultTimer;
if (key != null && key.ToLowerInvariant().StartsWith("otpauth://")) if (key != null && key.ToLowerInvariant().StartsWith("otpauth://"))
{ {
var qsParams = CoreHelpers.GetQueryParams(key); var qsParams = CoreHelpers.GetQueryParams(key);

View file

@ -2,7 +2,9 @@
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using System.Text.RegularExpressions; using System.Text.RegularExpressions;
using System.Web;
using Bit.Core.Models.Domain; using Bit.Core.Models.Domain;
using Bit.Core.Services;
using Newtonsoft.Json; using Newtonsoft.Json;
namespace Bit.Core.Utilities namespace Bit.Core.Utilities
@ -182,26 +184,20 @@ namespace Bit.Core.Utilities
public static Dictionary<string, string> GetQueryParams(string urlString) public static Dictionary<string, string> GetQueryParams(string urlString)
{ {
var dict = new Dictionary<string, string>(); try
if (!Uri.TryCreate(urlString, UriKind.Absolute, out var uri) || string.IsNullOrWhiteSpace(uri.Query))
{ {
return dict; if (!Uri.TryCreate(urlString, UriKind.Absolute, out var uri) || string.IsNullOrWhiteSpace(uri.Query))
{
return new Dictionary<string, string>();
}
var queryStringNameValueCollection = HttpUtility.ParseQueryString(uri.Query);
return queryStringNameValueCollection.AllKeys.Where(k => k != null).ToDictionary(k => k, k => queryStringNameValueCollection[k]);
} }
var pairs = uri.Query.Substring(1).Split('&'); catch (Exception ex)
foreach (var pair in pairs)
{ {
var parts = pair.Split('='); LoggerHelper.LogEvenIfCantBeResolved(ex);
if (parts.Length < 1)
{
continue;
}
var key = System.Net.WebUtility.UrlDecode(parts[0]).ToLower();
if (!dict.ContainsKey(key))
{
dict.Add(key, parts[1] == null ? string.Empty : System.Net.WebUtility.UrlDecode(parts[1]));
}
} }
return dict; return new Dictionary<string, string>();
} }
public static string SerializeJson(object obj, bool ignoreNulls = false) public static string SerializeJson(object obj, bool ignoreNulls = false)