Deprecate the raw overload of GenRandomStr in favour of GenRandom.

The raw overload was almost always misused where GenRandom would
be better.

While we're making changes to this code switch the printable mode
to use a static array like Anope does.

4 files changed, 32 insertions, 8 deletions

diff --git a/src/helperfuncs.cpp b/src/helperfuncs.cpp
index 5ebb6e44d..5ccb8ed6c 100644
--- a/src/helperfuncs.cpp
+++ b/src/helperfuncs.cpp
@@ -443,13 +443,31 @@ std::string Time::ToString(time_t curtime, const char* format, bool utc)
 	return buffer;
 }
 
+std::string InspIRCd::GenRandomStr(size_t length) const
+{
+	static const char chars[] = {
+		'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
+		'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z',
+		'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M',
+		'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z',
+		'0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+	};
+
+	std::string buf;
+	buf.reserve(length);
+	for (size_t idx = 0; idx < length; ++idx)
+		buf.push_back(chars[GenRandomInt(std::size(chars))]);
+	return buf;
+}
+
 std::string InspIRCd::GenRandomStr(size_t length, bool printable) const
 {
+	if (printable)
+		return GenRandomStr(length);
+
+	// DEPRECATED
 	std::vector<char> str(length);
 	GenRandom(str.data(), length);
-	if (printable)
-		for (size_t i = 0; i < length; i++)
-			str[i] = 0x3F + (str[i] & 0x3F);
 	return std::string(str.data(), str.size());
 }
 
diff --git a/src/modules/extra/m_argon2.cpp b/src/modules/extra/m_argon2.cpp
index 30d3de06b..999bd7d82 100644
--- a/src/modules/extra/m_argon2.cpp
+++ b/src/modules/extra/m_argon2.cpp
@@ -92,7 +92,8 @@ public:
 
 	std::string GenerateRaw(const std::string& data) override
 	{
-		const std::string salt = ServerInstance->GenRandomStr(config.saltlen, false);
+		std::vector<char> salt(config.saltlen);
+		ServerInstance->GenRandom(salt.data(), salt.size());
 
 		size_t encodedLen = argon2_encodedlen(
 			config.iterations,
@@ -111,8 +112,8 @@ public:
 			config.threads,
 			data.c_str(),
 			data.length(),
-			salt.c_str(),
-			salt.length(),
+			salt.data(),
+			salt.size(),
 			raw_data.data(),
 			raw_data.size(),
 			encoded_data.data(),
diff --git a/src/modules/m_password_hash.cpp b/src/modules/m_password_hash.cpp
index f96e1afc4..63731834a 100644
--- a/src/modules/m_password_hash.cpp
+++ b/src/modules/m_password_hash.cpp
@@ -56,7 +56,9 @@ public:
 				return CmdResult::FAILURE;
 			}
 
-			std::string salt = ServerInstance->GenRandomStr(hp->out_size, false);
+			std::string salt(hp->out_size, '\0');
+			ServerInstance->GenRandom(salt.data(), salt.length());
+
 			std::string target = hp->hmac(salt, parameters[1]);
 			std::string str = Base64::Encode(salt) + "$" + Base64::Encode(target, nullptr, 0);
 
diff --git a/src/modules/m_pbkdf2.cpp b/src/modules/m_pbkdf2.cpp
index c715659f0..63808804b 100644
--- a/src/modules/m_pbkdf2.cpp
+++ b/src/modules/m_pbkdf2.cpp
@@ -119,7 +119,10 @@ public:
 
 	std::string GenerateRaw(const std::string& data) override
 	{
-		PBKDF2Hash hs(this->iterations, this->dkey_length, ServerInstance->GenRandomStr(dkey_length, false));
+		std::string salt(dkey_length, '\0');
+		ServerInstance->GenRandom(salt.data(), salt.length());
+
+		PBKDF2Hash hs(this->iterations, this->dkey_length, salt);
 		hs.hash = PBKDF2(data, hs.salt, this->iterations, this->dkey_length);
 		return hs.ToString();
 	}