Насколько Хорошо Ваши Пароли Защищены? Проверка Проекта Bitwarden

Bitwarden — менеджер паролей с открытым исходным кодом.

Это программное обеспечение помогает создавать уникальные пароли и управлять ими.

Сможет ли анализатор PVS-Studio найти ошибки в таком проекте?

Насколько хорошо ваши пароли защищены? Проверка проекта Bitwarden

Введение

Менеджер паролей — это решение для хранения и генерации паролей.

Любой, кто использует такое программное обеспечение, хочет быть уверенным, что его данные в безопасности.

Качество кода такого инструмента имеет большое значение.

Поэтому я решил взять исходники Bitwarden от 15.03.2022 с сайта репозиторий и проверьте его с помощью статического анализатора PVS-Studio. Анализатор выдал 247 предупреждений по коду проекта.

Среди них мне удалось найти кое-что интересное.

Дополнительное задание

Выпуск 1

   

 public class BillingInvoice
 {
   public BillingInvoice(Invoice inv)
   {
     Amount = inv.AmountDue / 100M;      // <=
     Date = inv.Created;
     Url = inv.HostedInvoiceUrl;
     PdfUrl = inv.InvoicePdf;
     Number = inv.Number;
     Paid = inv.Paid;
     Amount = inv.Total / 100M;          // <=
   }
   public decimal Amount { get; set; }
   public DateTime? Date { get; set; }
   public string Url { get; set; }
   public string PdfUrl { get; set; }
   public string Number { get; set; }
   public bool Paid { get; set; }
 }

Предупреждение PVS-Studio: В3008 Переменной «Сумма» присваиваются значения два раза подряд. Возможно, это ошибка.

Строки проверки: 148, 142. BillingInfo.cs 148 Обратите внимание на инициализацию Количество .

Этому свойству присвоено выражение инв.

AmountDue/100M .

Необычно выглядит то, что буквально через пять строк кода выполняется аналогичная операция, но с присваиванием инв.

Всего/100M .

Трудно сказать, какое именно значение хотел использовать разработчик.

Если последнее присваивание истинно, то первое избыточно.

Красоты коду это не добавляет, но на логику выполнения программы все равно не влияет. В противном случае этот участок кода не будет работать корректно.

Логические ошибки

Выпуск 2

 private async Task<AppleReceiptStatus> GetReceiptStatusAsync(
   .

,
   AppleReceiptStatus lastReceiptStatus = null)
 {
   try
   {
     if (attempt > 4)
     {
       throw new Exception("Failed verifying Apple IAP " +
       "after too many attempts. " +
       "Last attempt status: " +
       lastReceiptStatus?.

Status ?? "null");          // <=
     }
     .


   }
   .

}

Предупреждение PVS-Studio: В3123 Возможно, '??' оператор работает не так, как ожидалось.

Его приоритет ниже приоритета других операторов в левой части.

AppleIapService.cs 96 Похоже, разработчик ожидал, что в сообщение будет добавлено значение свойства.

Положение дел или строка «ноль».

После чего результат будет добавлен в строку «Не удалось проверить Apple IAP после слишком большого количества попыток.

Статус последней попытки:».

К сожалению, поведение программы будет другим.

Чтобы понять суть этой операции, стоит вспомнить приоритеты операторов.

Оператор '??' имеет более низкий приоритет, чем оператор «+».

Поэтому строка со значением свойства будет добавлена первой.

Положение дел , и только после этого заработает оператор объединения нулей.

Если последнийReceiptStatus Нет нулевой И Положение дел Нет нулевой , этот метод работает правильно.

Если последнийReceiptStatus или Положение дел после всего нулевой , отобразится следующее сообщение: «Не удалось проверить Apple IAP после слишком большого количества попыток.

Статус последней попытки: «.

Это очевидно неверно.

Ожидаемое сообщение выглядит следующим образом: «Не удалось проверить Apple IAP после слишком большого количества попыток.

Статус последней попытки: нулевой».

Чтобы исправить ошибку, нужно часть выражения заключить в скобки:

throw new Exception("Failed verifying Apple IAP " + "after too many attempts. " + "Last attempt status: " + (lastReceiptStatus?.

Status ?? "null"));

Выпуск 3, 4

 public bool Validate(GlobalSettings globalSettings)
 {
   if(!(License == null && !globalSettings.SelfHosted) ||
      (License != null && globalSettings.SelfHosted))          // <=
   {
     return false;
   }
   return globalSettings.SelfHosted || !string.IsNullOrWhiteSpace(Country);
 }

Здесь PVS-Studio выдает сразу два предупреждения:

В3063 Часть условного выражения всегда является ложной, если она оценивается: globalSettings.SelfHosted. Премиумреквестмодел.
cs 23
В3063 Часть условного выражения всегда является ложной, если она оценивается: License != null. Премиумреквестмодел.
cs 23

Логическая часть выражения всегда будет ложной.

Чтобы убедиться в этом, следует рассмотреть возможные комбинации значений в условии:

Если Лицензия не равный нулевой , то левый операнд оператора '||' – истинный .
Правый операнд не будет оцениваться;
Если globalSettings.SelfHosted воля истинный , то левый операнд оператора '||' – истинный .
Правый операнд не будет оцениваться;
Если Лицензия равно нулевой , то правый операнд оператора '||' – ЛОЖЬ ;
Если globalSettings.SelfHosted воля ЛОЖЬ , то правый операнд оператора '||' – ЛОЖЬ ;

Оказывается, второй операнд '||' оператор либо не проверяется вообще, либо будет равен ЛОЖЬ .

Следовательно, это не влияет на истинность всего условия.

Часть условия после '||' является излишним.

Скорее всего, такой формат записи был выбран из соображений читабельности, но получилось немного странно.

Возможно, здесь следует проверить что-то еще.

Выпуск 5

 internal async Task DoRemoveSponsorshipAsync(
   Organization sponsoredOrganization,
   OrganizationSponsorship sponsorship = null)
 {
   .

sponsorship.SponsoredOrganizationId = null; sponsorship.FriendlyName = null; sponsorship.OfferedToEmail = null; sponsorship.PlanSponsorshipType = null; sponsorship.TimesRenewedWithoutValidation = 0; sponsorship.SponsorshipLapsedDate = null; // <= if (sponsorship.CloudSponsor || sponsorship.SponsorshipLapsedDate.HasValue) { await _organizationSponsorshipRepository.DeleteAsync(sponsorship); } else { await _organizationSponsorshipRepository.UpsertAsync(sponsorship); } }

Предупреждение PVS-Studio: В3063 Часть условного выражения всегда является ложной, если она оценивается: спонсорство.

SponsorshipLapsedDate.HasValue. ОрганизацияSponsorshipService.cs 308 Сообщение анализатора указывает, что часть логического условия всегда ложна.

Обратите внимание на инициализацию спонсорство.

SponsorshipLapsedDate .

Разработчик присваивает это свойство нулевой , после чего условие проверяет значение Хасвалуе у него дома.

Странно, что проверка происходит сразу после инициализации.

Это может иметь смысл, если свойство спонсорство.

CloudSponsor изменил значение спонсорство.

SponsorshipLapsedDate , но это неправда.

спонсорство.

CloudSponsor - обычное свойство auto:

public class OrganizationSponsorship : ITableObject<Guid> { .

public bool CloudSponsor { get; set; } .

}

Возможно, проверку реализуют на будущее, но сейчас это выглядит странно.

Проблемы с нулем

Выпуск 6

 public async Task ImportCiphersAsync(
   List<Folder> folders,
   List<CipherDetails> ciphers,
   IEnumerable<KeyValuePair<int, int>> folderRelationships)
 {
   var userId = folders.FirstOrDefault()?.

UserId ??
                ciphers.FirstOrDefault()?.

UserId;
 
   var personalOwnershipPolicyCount = 
     await _policyRepository
           .

GetCountByTypeApplicableToUserIdAsync(userId.Value, .

);
   .

if (userId.HasValue) { await _pushService.PushSyncVaultAsync(userId.Value); } }

Предупреждение PVS-Studio: В3095 Объект userId использовался до того, как было проверено его соответствие нулю.

Проверить строки: 640, 683. CipherService.cs 640 Чтобы представить суть триггеринга, стоит отметить, что переменная ID пользователя является объектом типа, допускающего значение NULL. Обратите внимание на этот фрагмент кода:

if (userId.HasValue) { await _pushService.PushSyncVaultAsync(userId.Value); }

Прежде чем связаться userId.Value .

проверки разработчика userId.HasValue .

Скорее всего, он предположил, что проверяемая величина может быть равна ЛОЖЬ.

Перед этим сообщением было еще одно:

_policyRepository.GetCountByTypeApplicableToUserIdAsync(userId.Value, .

);

Здесь мы также ссылаемся на userId.Value , но проверяет userId.HasValue почему-то нет. Или разработчик забыл проверить Хасвалуе первый раз или сделал ненужную проверку во второй раз.

Давайте разберемся, какой вариант правильный.

Для этого рассмотрим инициализацию ID пользователя :

var userId = folders.FirstOrDefault()?.

UserId ?? ciphers.FirstOrDefault()?.

UserId;

Из кода видно, что оба операнда '??' оператор может принимать значение типа, допускающего значение NULL, свойство которого Хасвалуе будет равен ЛОЖЬ .

Следовательно, userId.HasValue может иметь значение ЛОЖЬ .

Оказывается, при первом доступе userId.Value все еще стоит проверить userId.HasValue .

Ведь если стоимость имущества Хасвалуе равно ЛОЖЬ , апеллировать к Ценить одна и та же переменная приведет к выдаче исключения типа ИнвалидОператионИсключение.

Выпуск 7

 public async Task<List<OrganizationUser>> InviteUsersAsync(
   Guid organizationId,
   Guid? invitingUserId,
   IEnumerable<(OrganizationUserInvite invite, string externalId)> invites)
 {
   var organization = await GetOrgById(organizationId);
   var initialSeatCount = organization.Seats;
   if (organization == null || invites.Any(i => i.invite.Emails == null))
   {
     throw new NotFoundException();
   }
   .

}

Предупреждение PVS-Studio: В3095 Объект «организация» использовался до того, как он был проверен на нулевое значение.

Проверить строки: 1085, 1086. OrganizationService.cs 1085 Состояние проверено организация за равенство нулевой .

Оказывается, разработчик предполагал, что эта переменная может быть равна нулевой .

Кроме того, перед условием осуществляется доступ к свойству Сиденья переменная организация без всякой проверки нулевой .

Если организация – нулевой , этот вызов приведет к выдаче исключения типа Исключение NullReferenceException.

Выпуск 8

 public async Task<SubscriptionInfo> GetSubscriptionAsync(
   ISubscriber subscriber)
 {
   .


   if (!string.IsNullOrWhiteSpace(subscriber.GatewaySubscriptionId))
   {
     var sub = await _stripeAdapter.SubscriptionGetAsync(
       subscriber.GatewaySubscriptionId);
     
     if (sub != null)
     {
       subscriptionInfo.Subscription = 
         new SubscriptionInfo.BillingSubscription(sub);
     }
 
     if (   !sub.CanceledAt.HasValue
         && !string.IsNullOrWhiteSpace(subscriber.GatewayCustomerId))
     {
       .

} } return subscriptionInfo; }

Предупреждение PVS-Studio: В3125 Объект «sub» использовался после того, как было проверено его соответствие нулю.

Строки проверки: 1554, 1549. StripePaymentService.cs 1554 Анализатор сообщает о возможном доступе к нулевой ссылке.

Прежде чем передать переменную суб в конструктор SubscriptionInfo.BillingSubscription , разработчик проверяет его на наличие нулевой .

Странно, что сразу после этого без всякой проверки происходит доступ к свойству Отменено в эта переменная.

Такой вызов может привести к выдаче исключения, например Исключение NullReferenceException .

Выпуск 9

 public class FreshdeskController : Controller
 {
   .


   public FreshdeskController(
     IUserRepository userRepository,
     IOrganizationRepository organizationRepository,
     IOrganizationUserRepository organizationUserRepository,
     IOptions<BillingSettings> billingSettings,
     ILogger<AppleController> logger,
     GlobalSettings globalSettings)
   {
     _billingSettings = billingSettings?.

Value;                   // <=
     _userRepository = userRepository;
     _organizationRepository = organizationRepository;
     _organizationUserRepository = organizationUserRepository;
     _logger = logger;
     _globalSettings = globalSettings;
     _freshdeskAuthkey = Convert.ToBase64String(
           Encoding.UTF8
           .

GetBytes($"{_billingSettings.FreshdeskApiKey}:X"));   // <=
   }
   .

}

Предупреждение PVS-Studio: В3105 Переменная «_billingSettings» использовалась после того, как она была назначена с помощью условного оператора с нулевым значением.

Возможно исключение NullReferenceException. FreshdeskController.cs 47 Обратите внимание на инициализацию полей _billingSettings .

Вы заметите, что ему присвоено значение свойства Ценить , полученный с помощью условного оператора NULL. Скорее всего, ожидается, что Настройки биллинга может иметь значение нулевой .

Итак, в поле _billingSettings также можно назначить нулевой .

После инициализации _billingSettings осуществляется доступ к свойству FreshdeskApiKey :

_freshdeskAuthkey = Convert.ToBase64String( Encoding.UTF8 .

GetBytes($"{_billingSettings.FreshdeskApiKey}:X"));

Этот вызов может привести к выдаче исключения, например Исключение NullReferenceException.

Выпуск 10

 public PayPalIpnClient(IOptions<BillingSettings> billingSettings)
 {
   var bSettings = billingSettings?.

Value; _ipnUri = new Uri(bSettings.PayPal.Production ? " https://www.paypal.com/cgi-bin/webscr " : " https://www.sandbox.paypal.com/cgi-bin/webscr "); }

Предупреждение PVS-Studio: В3105 Переменная bSettings использовалась после того, как она была присвоена с помощью условного оператора с нулевым значением.

Возможно исключение NullReferenceException. PayPalIpnClient.cs 22 Запись, аналогичная предыдущей, находится в реализации метода PayPalIpnКлиент .

Здесь переменная бНастройки присваивается значение, полученное с помощью оператора с нулевым условием.

Далее осуществляется доступ к свойству PayPal та же переменная.

Такой вызов может привести к выдаче исключения, например Исключение NullReferenceException .

Выпуск 11

 public async Task<PagedResult<IEvent>> GetManyAsync(
   .

,
   PageOptions pageOptions)
 {
   .


   var query = new TableQuery<EventTableEntity>()
                   .

Where(filter)
                   .

Take(pageOptions.PageSize);                        // <=
   var result = new PagedResult<IEvent>();
   var continuationToken = DeserializeContinuationToken(
                             pageOptions?.

ContinuationToken);          // <=
   .

}

Предупреждение PVS-Studio: В3095 Объект «pageOptions» использовался до того, как было проверено его соответствие нулю.

Проверить строки: 135, 137. EventRepository.cs 135 Еще одна странность, связанная с отсутствием проверки на нулевой .

Доступ к переменной Параметры страницы делается дважды.

Во втором вызове используется оператор с нулевым условием, а в первом по какой-то причине — нет. Либо разработчик выполнил ненужные проверки нулевой во втором случае или забыл проверить Параметры страницы Во-первых.

Если второе предположение верно, то возможен доступ к нулевой ссылке, что приведет к исключению типа Исключение NullReferenceException.

Выпуск 12

 public async Task<string> PurchaseOrganizationAsync(.

, TaxInfo taxInfo)
 {
   .


   if (taxInfo != null &&                                             // <=
       !string.IsNullOrWhiteSpace(taxInfo.BillingAddressCountry) &&
       !string.IsNullOrWhiteSpace(taxInfo.BillingAddressPostalCode))
   {
     .


   }
   .


   Address = new Stripe.AddressOptions
   {
     Country = taxInfo.BillingAddressCountry,                         // <=
     PostalCode = taxInfo.BillingAddressPostalCode,
     Line1 = taxInfo.BillingAddressLine1 ?? string.Empty,
     Line2 = taxInfo.BillingAddressLine2,
     City = taxInfo.BillingAddressCity,
     State = taxInfo.BillingAddressState,
   }
   .

}

Предупреждение PVS-Studio: В3125 Объект «taxInfo» использовался после проверки на нулевое значение.

Проверить строки: 135, 99. StripePaymentService.cs 135 Анализатор в очередной раз нашел место, где могло произойти разыменование нулевой ссылки.

Действительно, странно выглядит, что переменная проверяется в условии налогИнформация на нулевой , но при нескольких обращениях к одной и той же переменной проверка не происходит.

Выпуск 13

 public IQueryable<OrganizationUserUserDetails> Run(DatabaseContext dbContext)
 {
   .

return query.Select(x => new OrganizationUserUserDetails { Id = x.ou.Id, OrganizationId = x.ou.OrganizationId, UserId = x.ou.UserId, Name = x.u.Name, // <= Email = x.u.Email ?? x.ou.Email, // <= TwoFactorProviders = x.u.TwoFactorProviders, // <= Premium = x.u.Premium, // <= Status = x.ou.Status, Type = x.ou.Type, AccessAll = x.ou.AccessAll, ExternalId = x.ou.ExternalId, SsoExternalId = x.su.ExternalId, Permissions = x.ou.Permissions, ResetPasswordKey = x.ou.ResetPasswordKey, UsesKeyConnector = x.u != null && x.u.UsesKeyConnector, // <= }); }

Предупреждение PVS-Studio: В3095 Объект «x.u» использовался до того, как он был проверен на ноль.

Проверьте строки: 24, 32. OrganizationUserUserViewQuery.cs 24 Необычно то, что переменная х.

у.

по сравнению с нулевой , потому что до этого к его свойствам обращались (и не раз!).

Возможно, это просто ненужная проверка.

Также существует вероятность того, что разработчик забыл проверить нулевой перед назначением.

Неправильный постфикс

Выпуск 14

 private async Task<HttpResponseMessage> CallFreshdeskApiAsync(
   HttpRequestMessage request,
   int retriedCount = 0)
 {
   try
   {
     request.Headers.Add("Authorization", _freshdeskAuthkey);
     var response = await _httpClient.SendAsync(request);
     if (   response.StatusCode != System.Net.HttpStatusCode.TooManyRequests
         || retriedCount > 3)
     {
       return response;
     }
   }
   catch
   {
     if (retriedCount > 3)
     {
       throw;
     }
   }
   await Task.Delay(30000 * (retriedCount + 1));
   return await CallFreshdeskApiAsync(request, retriedCount++);    // <=
 }

Предупреждение PVS-Studio: В3159 Измененное значение операнда retriedCount не используется после операции увеличения постфикса.

FreshdeskController.cs 167 Обратите внимание на приращение переменной retriedCount .

Очень странно, что в данном случае используется постфиксная нотация.

Ведь сначала возвращается текущее значение переменной, и только потом это значение увеличивается.

Возможно, стоит изменить постфиксную запись на префиксную:

return await CallFreshdeskApiAsync(request, ++retriedCount)

Для большей наглядности можно использовать следующие обозначения:

return await CallFreshdeskApiAsync(request, retriedCount + 1)

Заключение

Вероятно, среди описанных дефектов нет ничего, что представляло бы угрозу безопасности.

Большинство ошибок сводится к возможности возникновения исключений при работе с нулевыми ссылками.

Однако эти области заслуживают улучшения.

Как было сказано в начале статьи, даже из сравнительно небольшого количества ответов анализатора можно выделить интересные моменты.

Возможно, некоторые недостатки не влияют на работу программы, но их тоже следует избегать.

Хотя бы для того, чтобы у других разработчиков не возникало лишних вопросов.

Я считаю очень удобным иметь инструмент, позволяющий быстро находить дефекты в коде.

Как видите, таким инструментом может стать статический анализатор :).

Я предлагаю это вам бесплатно попробуйте PVS-Studio чтобы увидеть, какие ошибки скрыты в интересующем вас проекте.

Если вы хотите поделиться этой статьей с англоязычной аудиторией, воспользуйтесь ссылкой для перевода: Никита Паневин.

Вы уверены, что ваши пароли защищены? Проверка проекта Bitwarden .

Теги: #C++ #.

NET #статический анализ кода #pvs-studio

Насколько Хорошо Ваши Пароли Защищены? Проверка Проекта Bitwarden

Последнее изменение: 2024-10-19 21:10:22

Вместе с данным постом часто просматривают:

Книги Для Работы В It-Компаниях
19 Oct, 24
Файл Humans.txt От Google
19 Oct, 24
Лечение Синдрома Двойного Щелчка
19 Oct, 24
Пальцы Робота Делают Глупые Устройства Умными
19 Oct, 24
Открытый Интерфейс Hive: Мы Сделали Это
19 Oct, 24
Управление Конфигурацией
19 Oct, 24
Облачный Токен Pkcs#11 – Миф Или Реальность?
19 Oct, 24
История Ит Бжд
19 Oct, 24
В Штатах Любят Coca-Cola, А В Британии Любят Microsoft
19 Oct, 24
Дайджест Интересных Материалов Из Мира Веб-Разработки И It За Прошедшую Неделю №185 (8 – 15 Ноября 2015 Г.)
19 Oct, 24