CVE-2019-12816: Improper Input Validation (CWE-20)
value: Modules.cpp in ZNC before 1.7.4-rc1 allows remote authenticated non-admin users to escalate privileges and execute arbitrary code by loading a module with a crafted name.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 8.8
Published: 2019-06-15T16:29Z
Repository: https://github.com/znc/znc
Language: C++
Files changed: include/znc/Modules.h, src/Modules.cpp
Commit date: 2019-06-15 02:00:42 +0100
Commit message: 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

Diff: 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
@@ -1598,10 +1598,11 @@ class CModules : public std::vector<CModule*>, private CCoreTranslationMixin {
     // !Global Modules
 
   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;
     CIRCNetwork* m_pNetwork;
     CClient* m_pClient;
diff --git a/src/Modules.cpp b/src/Modules.cpp
index 5aec7805a..d41951a8d 100644
--- a/src/Modules.cpp
+++ b/src/Modules.cpp
@@ -1622,15 +1622,34 @@ 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;
     }
 
@@ -1779,10 +1798,14 @@ bool CModules::ReloadModule(const CString& sModule, const CString& sArgs,
     return true;
 }
 
 bool CModules::GetModInfo(CModInfo& ModInfo, const CString& sModule,
                           CString& sRetMsg) {
+    if (!ValidateModuleName(sModule, sRetMsg)) {
+        return false;
+    }
+
     CString sModPath, sTmp;
 
     bool bSuccess;
     bool bHandled = false;
     GLOBALMODULECALL(OnGetModInfo(ModInfo, sModule, bSuccess, sRetMsg),
@@ -1797,10 +1820,14 @@ bool CModules::GetModInfo(CModInfo& ModInfo, const CString& sModule,
     return GetModPathInfo(ModInfo, sModule, sModPath, sRetMsg);
 }
 
 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);
 
     ModHandle p = OpenModule(sModule, sModPath, ModInfo, sRetMsg);
     if (!p) return false;
@@ -1909,19 +1936,12 @@ CModules::ModDirList CModules::GetModDirs() {
 ModHandle CModules::OpenModule(const CString& sModule, const CString& sModPath,
                                CModInfo& Info, CString& sRetMsg) {
     // 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
     // that (despite what the man page says) we must include either of
     // RTLD_NOW and RTLD_LAZY and either of RTLD_GLOBAL and RTLD_LOCAL.

Vulnerable code:     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;
        }
Fixed code:     static bool ValidateModuleName(const CString& sModule, CString& sRetMsg);
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;
}

    if (!ValidateModuleName(sModule, sRetMsg)) {
        return false;
    }

    if (!ValidateModuleName(sModule, sRetMsg)) {
        return false;
    }

    if (!ValidateModuleName(sModule, sRetMsg)) {
        return false;
    }

    if (!ValidateModuleName(sModule, sRetMsg)) {
        return nullptr;