CVE-2022-28927: Unrestricted Upload of File with Dangerous Type (CWE-434)
value: A remote code execution (RCE) vulnerability in Subconverter v0.7.2 allows attackers to execute arbitrary code via crafted config and url parameters.
CWE description: The product allows the upload or transfer of dangerous file types that are automatically processed within its environment.
CVSS v3 base score: 9.8
Published: 2022-05-19T16:15Z
Repository: https://github.com/tindy2013/subconverter
Language: C++
Files changed: src/generator/config/nodemanip.cpp, src/generator/config/subexport.cpp, src/generator/config/subexport.h, src/handler/interfaces.cpp, src/handler/settings.cpp, src/handler/settings.h
Commit date: 2022-04-04 00:38:44 +0800
Commit message: Enhancements

Add authorization check before loading scripts.
Add detailed logs when loading preference settings.

Diff: diff --git a/src/generator/config/nodemanip.cpp b/src/generator/config/nodemanip.cpp
index 7684742..6d6d425 100644
--- a/src/generator/config/nodemanip.cpp
+++ b/src/generator/config/nodemanip.cpp
@@ -49,11 +49,11 @@ int addNodes(std::string link, std::vector<Proxy> &allNodes, int groupID, parse_
 
     // TODO: replace with startsWith if appropriate
     link = replaceAllDistinct(link, "\"", "");
 
     /// script:filepath,arg1,arg2,...
-    script_safe_runner(parse_set.js_runtime, parse_set.js_context, [&](qjs::Context &ctx)
+    if(authorized) script_safe_runner(parse_set.js_runtime, parse_set.js_context, [&](qjs::Context &ctx)
     {
         if(startsWith(link, "script:")) /// process subscription with script
         {
             writeLog(0, "Found script link. Start running...", LOG_LEVEL_INFO);
             string_array args = split(link.substr(7), ",");
@@ -378,11 +378,11 @@ void nodeRename(Proxy &node, const RegexMatchConfigs &rename_array, extra_settin
 {
     std::string &remark = node.Remark, original_remark = node.Remark, returned_remark, real_rule;
 
     for(const RegexMatchConfig &x : rename_array)
     {
-        if(!x.Script.empty())
+        if(!x.Script.empty() && ext.authorized)
         {
             script_safe_runner(ext.js_runtime, ext.js_context, [&](qjs::Context &ctx)
             {
                 std::string script = x.Script;
                 if(startsWith(script, "path:"))
@@ -430,11 +430,11 @@ std::string addEmoji(const Proxy &node, const RegexMatchConfigs &emoji_array, ex
 {
     std::string real_rule, ret;
 
     for(const RegexMatchConfig &x : emoji_array)
     {
-        if(!x.Script.empty())
+        if(!x.Script.empty() && ext.authorized)
         {
             std::string result;
             script_safe_runner(ext.js_runtime, ext.js_context, [&](qjs::Context &ctx)
             {
                 std::string script = x.Script;
@@ -479,11 +479,11 @@ void preprocessNodes(std::vector<Proxy> &nodes, extra_settings &ext)
     });
 
     if(ext.sort_flag)
     {
         bool failed = true;
-        if(ext.sort_script.size())
+        if(ext.sort_script.size() && ext.authorized)
         {
             std::string script = ext.sort_script;
             if(startsWith(script, "path:"))
                 script = fileGet(script.substr(5), false);
             script_safe_runner(ext.js_runtime, ext.js_context, [&](qjs::Context &ctx)
diff --git a/src/generator/config/subexport.cpp b/src/generator/config/subexport.cpp
index f01f22b..27576f1 100644
--- a/src/generator/config/subexport.cpp
+++ b/src/generator/config/subexport.cpp
@@ -181,11 +181,11 @@ void groupGenerate(const std::string &rule, std::vector<Proxy> &nodelist, string
     if(startsWith(rule, "[]") && add_direct)
     {
         filtered_nodelist.emplace_back(rule.substr(2));
     }
 #ifndef NO_JS_RUNTIME
-    else if(startsWith(rule, "script:"))
+    else if(startsWith(rule, "script:") && ext.authorized)
     {
         script_safe_runner(ext.js_runtime, ext.js_context, [&](qjs::Context &ctx){
             std::string script = fileGet(rule.substr(7), true);
             try
             {
diff --git a/src/generator/config/subexport.h b/src/generator/config/subexport.h
index 6095810..dce5bc5 100644
--- a/src/generator/config/subexport.h
+++ b/src/generator/config/subexport.h
@@ -37,10 +37,11 @@ struct extra_settings
     tribool skip_cert_verify = tribool();
     tribool tls13 = tribool();
     bool clash_classical_ruleset = false;
     std::string sort_script = "";
     std::string clash_proxies_style = "flow";
+    bool authorized = false;
 
     extra_settings() {};
     extra_settings(const extra_settings&) = delete;
     extra_settings(extra_settings&&) = delete;
 
diff --git a/src/handler/interfaces.cpp b/src/handler/interfaces.cpp
index 80dacbd..c299421 100644
--- a/src/handler/interfaces.cpp
+++ b/src/handler/interfaces.cpp
@@ -405,10 +405,11 @@ std::string subconverter(RESPONSE_CALLBACK_ARGS)
 
     /// check for proxy settings
     std::string proxy = parseProxy(global.proxySubscription);
 
     /// check other flags
+    ext.authorized = authorized;
     ext.append_proxy_type = argAppendType.get(global.appendType);
     if((argTarget == "clash" || argTarget == "clashr") && argGenClashScript.is_undef())
         argExpandRulesets.define(true);
 
     ext.clash_proxies_style = global.clashProxiesStyle;
diff --git a/src/handler/settings.cpp b/src/handler/settings.cpp
index 398afcb..65192e2 100644
--- a/src/handler/settings.cpp
+++ b/src/handler/settings.cpp
@@ -575,10 +575,11 @@ void readYAMLConf(YAML::Node &node)
         }
         node["advanced"]["script_clean_context"] >> global.scriptCleanContext;
         node["advanced"]["async_fetch_ruleset"] >> global.asyncFetchRuleset;
         node["advanced"]["skip_failed_links"] >> global.skipFailedLinks;
     }
+    writeLog(0, "Load preference settings in YAML format completed.", LOG_LEVEL_INFO);
 }
 
 template <class T, class... U>
 void find_if_exist(const toml::value &v, const toml::key &k, T& target, U&&... args)
 {
@@ -784,17 +785,18 @@ void readTOMLConf(toml::value &root)
     }
     else
     {
         global.cacheSubscription = global.cacheConfig = global.cacheRuleset = 0;
     }
+
+    writeLog(0, "Load preference settings in TOML format completed.", LOG_LEVEL_INFO);
 }
 
 void readConf()
 {
     guarded_mutex guard(gMutexConfigure);
-    //std::cerr<<"Reading preference settings..."<<std::endl;
-    writeLog(0, "Reading preference settings...", LOG_LEVEL_INFO);
+    writeLog(0, "Loading preference settings...", LOG_LEVEL_INFO);
 
     eraseElements(global.excludeRemarks);
     eraseElements(global.includeRemarks);
     eraseElements(global.customProxyGroups);
     eraseElements(global.customRulesets);
@@ -813,25 +815,27 @@ void readConf()
             return readTOMLConf(conf);
     }
     catch (YAML::Exception &e)
     {
         //ignore yaml parse error
+        writeLog(0, e.what(), LOG_LEVEL_DEBUG);
+        writeLog(0, "Unable to load preference settings as YAML.", LOG_LEVEL_DEBUG);
     }
     catch (toml::exception &e)
     {
         //ignore toml parse error
         writeLog(0, e.what(), LOG_LEVEL_DEBUG);
+        writeLog(0, "Unable to load preference settings as TOML.", LOG_LEVEL_DEBUG);
     }
 
     INIReader ini;
     ini.allow_dup_section_titles = true;
     //ini.do_utf8_to_gbk = true;
     int retVal = ini.ParseFile(global.prefPath);
     if(retVal != INIREADER_EXCEPTION_NONE)
     {
-        //std::cerr<<"Unable to load preference settings. Reason: "<<ini.GetLastError()<<"\n";
-        writeLog(0, "Unable to load preference settings. Reason: " + ini.GetLastError(), LOG_LEVEL_FATAL);
+        writeLog(0, "Unable to load preference settings as INI. Reason: " + ini.GetLastError(), LOG_LEVEL_FATAL);
         return;
     }
 
     string_array tempArray;
 
@@ -1068,12 +1072,11 @@ void readConf()
     }
     ini.GetBoolIfExist("script_clean_context", global.scriptCleanContext);
     ini.GetBoolIfExist("async_fetch_ruleset", global.asyncFetchRuleset);
     ini.GetBoolIfExist("skip_failed_links", global.skipFailedLinks);
 
-    //std::cerr<<"Read preference settings completed."<<std::endl;
-    writeLog(0, "Read preference settings completed.", LOG_LEVEL_INFO);
+    writeLog(0, "Load preference settings in INI format completed.", LOG_LEVEL_INFO);
 }
 
 int loadExternalYAML(YAML::Node &node, ExternalConfig &ext)
 {
     YAML::Node section = node["custom"], object;
diff --git a/src/handler/settings.h b/src/handler/settings.h
index d22e0b5..837ff17 100644
--- a/src/handler/settings.h
+++ b/src/handler/settings.h
@@ -27,11 +27,11 @@ struct Settings
     bool prependInsert = true, skipFailedLinks = false;
     bool APIMode = true, writeManagedConfig = false, enableRuleGen = true, updateRulesetOnRequest = false, overwriteOriginalRules = true;
     bool printDbgInfo = false, CFWChildProcess = false, appendUserinfo = true, asyncFetchRuleset = false, surgeResolveHostname = true;
     std::string accessToken, basePath = "base";
     std::string custom_group;
-    int logLevel = LOG_LEVEL_INFO;
+    int logLevel = LOG_LEVEL_VERBOSE;
     long maxAllowedDownloadSize = 1048576L;
     string_map aliases;
 
     //global variables for template
     std::string templatePath = "templates";

Vulnerable code:     script_safe_runner(parse_set.js_runtime, parse_set.js_context, [&](qjs::Context &ctx)
        if(!x.Script.empty())
        if(!x.Script.empty())
        if(ext.sort_script.size())
    else if(startsWith(rule, "script:"))
    //std::cerr<<"Reading preference settings..."<<std::endl;
    writeLog(0, "Reading preference settings...", LOG_LEVEL_INFO);
        //std::cerr<<"Unable to load preference settings. Reason: "<<ini.GetLastError()<<"\n";
        writeLog(0, "Unable to load preference settings. Reason: " + ini.GetLastError(), LOG_LEVEL_FATAL);
    //std::cerr<<"Read preference settings completed."<<std::endl;
    writeLog(0, "Read preference settings completed.", LOG_LEVEL_INFO);
    int logLevel = LOG_LEVEL_INFO;
Fixed code:     if(authorized) script_safe_runner(parse_set.js_runtime, parse_set.js_context, [&](qjs::Context &ctx)
        if(!x.Script.empty() && ext.authorized)
        if(!x.Script.empty() && ext.authorized)
        if(ext.sort_script.size() && ext.authorized)
    else if(startsWith(rule, "script:") && ext.authorized)
    bool authorized = false;
    ext.authorized = authorized;
    writeLog(0, "Load preference settings in YAML format completed.", LOG_LEVEL_INFO);

    writeLog(0, "Load preference settings in TOML format completed.", LOG_LEVEL_INFO);
    writeLog(0, "Loading preference settings...", LOG_LEVEL_INFO);
        writeLog(0, e.what(), LOG_LEVEL_DEBUG);
        writeLog(0, "Unable to load preference settings as YAML.", LOG_LEVEL_DEBUG);
        writeLog(0, "Unable to load preference settings as TOML.", LOG_LEVEL_DEBUG);
        writeLog(0, "Unable to load preference settings as INI. Reason: " + ini.GetLastError(), LOG_LEVEL_FATAL);
    writeLog(0, "Load preference settings in INI format completed.", LOG_LEVEL_INFO);
    int logLevel = LOG_LEVEL_VERBOSE;