Skip to content

Commit

Permalink
Fix sensitive data leaking in Authentication
Browse files Browse the repository at this point in the history
The current Authentication constructor has multiple points where a copy
can get made: in the arguments themselves, in the intermediate
concatenations, and in the potential need for the concatenation to copy
itself during a memory reallocation.

An additional copy of the auth data could end up unwiped in the implicit
move constructor/assignment (in particular when small string
optimization applies to the value).

Any such copies end up potentially leaving the sensitive data behind in
memory, undermining the changes in #776 that were trying to securely
erase such sensitive data.

This commit avoids any such copies by:
- changing Authentication to take string_views (instead of std::string)
  for username and password so that no copy of input will be done
- properly reserving auth_string_ to its required size before building
  it
- resizing the auth_string_ of moved-from values to their capacity so
  that secureStringClear will properly erase them.
- Adding an explicit move constructor that resizes the moved-from auth
  string to capacity to ensure it gets erased when SSO applies.
- Adding an explicit move assignment operator that wipes the current
  value before replacing it, and properly resizes the moved-from string
  to capacity to ensure it gets wiped when SSO applies.
  • Loading branch information
jagerman committed Jun 21, 2024
1 parent 923f83a commit b4ec38c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 5 deletions.
20 changes: 20 additions & 0 deletions cpr/auth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@
#include "cpr/util.h"

namespace cpr {

Authentication::Authentication(std::string_view username, std::string_view password, AuthMode auth_mode) : auth_mode_{auth_mode} {
auth_string_.reserve(username.size() + 1 + password.size());
auth_string_ += username;
auth_string_ += ':';
auth_string_ += password;
}

Authentication::Authentication(Authentication&& old) noexcept : auth_string_{std::move(old.auth_string_)}, auth_mode_{old.auth_mode_} {
old.auth_string_.resize(old.auth_string_.capacity());
}

Authentication& Authentication::operator=(Authentication&& old) noexcept {
auth_mode_ = old.auth_mode_;
util::secureStringClear(auth_string_);
auth_string_ = std::move(old.auth_string_);
old.auth_string_.resize(old.auth_string_.capacity());
return *this;
}

Authentication::~Authentication() noexcept {
util::secureStringClear(auth_string_);
}
Expand Down
9 changes: 4 additions & 5 deletions include/cpr/auth.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,20 @@
#define CPR_AUTH_H

#include <string>

#include <utility>
#include <string_view>

namespace cpr {

enum class AuthMode { BASIC, DIGEST, NTLM };

class Authentication {
public:
Authentication(std::string username, std::string password, AuthMode auth_mode) : auth_string_{std::move(username) + ":" + std::move(password)}, auth_mode_{std::move(auth_mode)} {}
Authentication(std::string_view username, std::string_view password, AuthMode auth_mode);
Authentication(const Authentication& other) = default;
Authentication(Authentication&& old) noexcept = default;
Authentication(Authentication&& old) noexcept;
~Authentication() noexcept;

Authentication& operator=(Authentication&& old) noexcept = default;
Authentication& operator=(Authentication&& old) noexcept;
Authentication& operator=(const Authentication& other) = default;

const char* GetAuthString() const noexcept;
Expand Down

0 comments on commit b4ec38c

Please sign in to comment.