From 8de9e376ce531fe7f3c8b0aa4876d15b479b7311 Mon Sep 17 00:00:00 2001
From: Alexey Sokolov <alexey+znc@asokolov.org>
Date: Wed, 12 Jun 2019 08:57:29 +0100
Subject: [PATCH] Fix remote code execution and privilege escalation
 vulnerability.

To trigger this, need to have a user already.

Thanks for Jeriko One <jeriko.one@gmx.us> for finding and reporting this.

CVE-2019-12816
---
 include/znc/Modules.h |  1 +
 src/Modules.cpp       | 38 +++++++++++++++++++++++++++++---------
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/include/znc/Modules.h b/include/znc/Modules.h
index 28fdd3a62..db8f87b81 100644
--- a/include/znc/Modules.h
+++ b/include/znc/Modules.h
@@ -1600,6 +1600,7 @@ class CModules : public std::vector<CModule*>, private CCoreTranslationMixin {
   private:
     static ModHandle OpenModule(const CString& sModule, const CString& sModPath,
                                 CModInfo& Info, CString& sRetMsg);
+    static bool ValidateModuleName(const CString& sModule, CString& sRetMsg);
 
   protected:
     CUser* m_pUser;
diff --git a/src/Modules.cpp b/src/Modules.cpp
index 5aec7805a..d41951a8d 100644
--- a/src/Modules.cpp
+++ b/src/Modules.cpp
@@ -1624,11 +1624,30 @@ CModule* CModules::FindModule(const CString& sModule) const {
     return nullptr;
 }
 
+bool CModules::ValidateModuleName(const CString& sModule, CString& sRetMsg) {
+    for (unsigned int a = 0; a < sModule.length(); a++) {
+        if (((sModule[a] < '0') || (sModule[a] > '9')) &&
+            ((sModule[a] < 'a') || (sModule[a] > 'z')) &&
+            ((sModule[a] < 'A') || (sModule[a] > 'Z')) && (sModule[a] != '_')) {
+            sRetMsg =
+                t_f("Module names can only contain letters, numbers and "
+                    "underscores, [{1}] is invalid")(sModule);
+            return false;
+        }
+    }
+
+    return true;
+}
+
 bool CModules::LoadModule(const CString& sModule, const CString& sArgs,
                           CModInfo::EModuleType eType, CUser* pUser,
                           CIRCNetwork* pNetwork, CString& sRetMsg) {
     sRetMsg = "";
 
+    if (!ValidateModuleName(sModule, sRetMsg)) {
+        return false;
+    }
+
     if (FindModule(sModule) != nullptr) {
         sRetMsg = t_f("Module {1} already loaded.")(sModule);
         return false;
@@ -1781,6 +1800,10 @@ bool CModules::ReloadModule(const CString& sModule, const CString& sArgs,
 
 bool CModules::GetModInfo(CModInfo& ModInfo, const CString& sModule,
                           CString& sRetMsg) {
+    if (!ValidateModuleName(sModule, sRetMsg)) {
+        return false;
+    }
+
     CString sModPath, sTmp;
 
     bool bSuccess;
@@ -1799,6 +1822,10 @@ bool CModules::GetModInfo(CModInfo& ModInfo, const CString& sModule,
 
 bool CModules::GetModPathInfo(CModInfo& ModInfo, const CString& sModule,
                               const CString& sModPath, CString& sRetMsg) {
+    if (!ValidateModuleName(sModule, sRetMsg)) {
+        return false;
+    }
+
     ModInfo.SetName(sModule);
     ModInfo.SetPath(sModPath);
 
@@ -1911,15 +1938,8 @@ ModHandle CModules::OpenModule(const CString& sModule, const CString& sModPath,
     // Some sane defaults in case anything errors out below
     sRetMsg.clear();
 
-    for (unsigned int a = 0; a < sModule.length(); a++) {
-        if (((sModule[a] < '0') || (sModule[a] > '9')) &&
-            ((sModule[a] < 'a') || (sModule[a] > 'z')) &&
-            ((sModule[a] < 'A') || (sModule[a] > 'Z')) && (sModule[a] != '_')) {
-            sRetMsg =
-                t_f("Module names can only contain letters, numbers and "
-                    "underscores, [{1}] is invalid")(sModule);
-            return nullptr;
-        }
+    if (!ValidateModuleName(sModule, sRetMsg)) {
+        return nullptr;
     }
 
     // The second argument to dlopen() has a long history. It seems clear