Skip to content

Commit 7060ed8

Browse files
Add additional security hardening for I2P
- Fix socket leak in Hello() if HELLO handshake fails - Add SAM protocol injection protection (validate destinations) - Add validation for empty private key file (regenerate if corrupted) - Add IsSafeSAMValue() to reject control characters in SAM responses Co-authored-by: reuben <[email protected]>
1 parent 5c3e082 commit 7060ed8

File tree

1 file changed

+40
-1
lines changed

1 file changed

+40
-1
lines changed

src/i2p.cpp

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,29 @@ static CNetAddr DestB64ToAddr(const std::string& dest)
126126
return DestBinToAddr(decoded);
127127
}
128128

129+
/**
130+
* Check if a string is safe to use in SAM protocol commands.
131+
* Rejects strings containing control characters (including newlines) that could
132+
* be used to inject additional SAM commands.
133+
* @param[in] s The string to validate.
134+
* @return true if safe, false if contains dangerous characters.
135+
*/
136+
static bool IsSafeSAMValue(const std::string& s)
137+
{
138+
for (char c : s) {
139+
// Reject control characters (0x00-0x1F) and DEL (0x7F)
140+
// This prevents newline injection attacks
141+
if (static_cast<unsigned char>(c) < 0x20 || c == 0x7F) {
142+
return false;
143+
}
144+
// Also reject spaces as they delimit SAM protocol tokens
145+
if (c == ' ') {
146+
return false;
147+
}
148+
}
149+
return true;
150+
}
151+
129152
namespace sam {
130153

131154
Session::Session(const boost::filesystem::path& private_key_file,
@@ -261,6 +284,11 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
261284
SendRequestAndGetReply(sock, strprintf("NAMING LOOKUP NAME=%s", to.ToStringIP()));
262285

263286
const std::string& dest = lookup_reply.Get("VALUE");
287+
288+
// Validate the destination to prevent SAM command injection attacks
289+
if (!IsSafeSAMValue(dest)) {
290+
throw std::runtime_error("SAM proxy returned invalid destination (contains control characters)");
291+
}
264292

265293
const Reply& connect_reply = SendRequestAndGetReply(
266294
sock, strprintf("STREAM CONNECT ID=%s DESTINATION=%s SILENT=false", session_id, dest),
@@ -352,7 +380,12 @@ SOCKET Session::Hello() const
352380
throw std::runtime_error(strprintf("Cannot connect to %s", m_control_host.ToString()));
353381
}
354382

355-
SendRequestAndGetReply(sock, "HELLO VERSION MIN=3.1 MAX=3.1");
383+
try {
384+
SendRequestAndGetReply(sock, "HELLO VERSION MIN=3.1 MAX=3.1");
385+
} catch (...) {
386+
CloseSocket(sock);
387+
throw;
388+
}
356389

357390
return sock;
358391
}
@@ -480,6 +513,12 @@ void Session::CreateIfNotCreatedAlready()
480513
std::istreambuf_iterator<char>(file),
481514
std::istreambuf_iterator<char>());
482515
file.close();
516+
517+
// Validate the private key file is not empty or corrupted
518+
if (m_private_key.empty()) {
519+
LogPrintf("I2P: Private key file %s is empty, regenerating\n", m_private_key_file.string());
520+
GenerateAndSavePrivateKey(sock);
521+
}
483522
} else {
484523
GenerateAndSavePrivateKey(sock);
485524
}

0 commit comments

Comments
 (0)