From cbb6e14c3abe79839f9fdb6cc099424e950331f8 Mon Sep 17 00:00:00 2001
From: psychon <psychon@726aef4b-f618-498e-8847-2d620e286838>
Date: Tue, 9 Mar 2010 19:44:24 +0000
Subject: [PATCH] Generate session IDs more securely

We now use a lot more data for generating the session id which is fed to a hash
to make it impossible to attack specific parts of the input.

Also we now retry generating a new session id in the (improbable) case of
collision with an existing session id.

Thanks a lot to cnu for pointing out the weakness in the old code by stealing my
session cookie, you evil hacker!


git-svn-id: https://znc.svn.sourceforge.net/svnroot/znc/trunk@1819 726aef4b-f618-498e-8847-2d620e286838
---
 WebModules.cpp | 22 ++++++++++++++--------
 WebModules.h   |  6 +++---
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/WebModules.cpp b/WebModules.cpp
index 2a178ad5..4358eb5f 100644
--- a/WebModules.cpp
+++ b/WebModules.cpp
@@ -27,11 +27,6 @@ bool CZNCTagHandler::HandleTag(CTemplate& Tmpl, const CString& sName, const CStr
 }
 
 CWebSession::CWebSession(const CString& sId) : m_sId(sId) {
-	if (m_sId.empty()) {
-		m_sId = CString::RandomString(32);
-		DEBUG("Auto generated session: [" + m_sId + "]");
-	}
-
 	m_bLoggedIn = false;
 	m_pUser = NULL;
 }
@@ -601,7 +596,7 @@ void CWebSock::PrintErrorPage(const CString& sMessage) {
 	m_Template["Error"] = sMessage;
 }
 
-CSmartPtr<CWebSession> CWebSock::GetSession() const {
+CSmartPtr<CWebSession> CWebSock::GetSession() {
 	if (!m_spSession.IsNull()) {
 		return m_spSession;
 	}
@@ -613,7 +608,18 @@ CSmartPtr<CWebSession> CWebSock::GetSession() const {
 		return it->second;
 	}
 
-	CSmartPtr<CWebSession> spSession(new CWebSession());
+	CString sSessionID;
+	do {
+		sSessionID = CString::RandomString(32);
+		sSessionID += ":" + GetRemoteIP() + ":" + CString(GetRemotePort());
+		sSessionID += ":" + GetLocalIP()  + ":" + CString(GetLocalPort());
+		sSessionID += ":" + CString(time(NULL));
+		sSessionID = sSessionID.SHA256();
+
+		DEBUG("Auto generated session: [" + sSessionID + "]");
+	} while (m_mspSessions.find(sSessionID) != m_mspSessions.end());
+
+	CSmartPtr<CWebSession> spSession(new CWebSession(sSessionID));
 	m_mspSessions.insert(make_pair(spSession->GetId(), spSession));
 
 	return spSession;
@@ -640,7 +646,7 @@ Csock* CWebSock::GetSockObj(const CString& sHost, unsigned short uPort) {
 	return pSock;
 }
 
-CString CWebSock::GetSkinName() const {
+CString CWebSock::GetSkinName() {
 	CSmartPtr<CWebSession> spSession = GetSession();
 
 	if (spSession->IsLoggedIn() && !spSession->GetUser()->GetSkinName().empty()) {
diff --git a/WebModules.h b/WebModules.h
index e8252e95..d3065a05 100644
--- a/WebModules.h
+++ b/WebModules.h
@@ -34,7 +34,7 @@ private:
 
 class CWebSession {
 public:
-	CWebSession(const CString& sId = "");
+	CWebSession(const CString& sId);
 	virtual ~CWebSession() {}
 
 	const CString& GetId() const { return m_sId; }
@@ -135,14 +135,14 @@ public:
 
 	void PrintErrorPage(const CString& sMessage);
 
-	CSmartPtr<CWebSession> GetSession() const;
+	CSmartPtr<CWebSession> GetSession();
 
 	virtual Csock* GetSockObj(const CString& sHost, unsigned short uPort);
 	CString GetModWebPath(const CString& sModName) const;
 	CString GetSkinPath(const CString& sSkinName) const;
 	CModule* GetModule() const { return (CModule*) m_pModule; }
 	size_t GetAvailSkins(vector<CFile>& vRet);
-	CString GetSkinName() const;
+	CString GetSkinName();
 
 	CString GetCookie(const CString& sKey) const;
 	bool SetCookie(const CString& sKey, const CString& sValue);