Adsk Contrib - Miscellaneous improvements suggested by Claude (#2308)

* Various fixes suggested by Claude

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Address review comments

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Update security guidelines

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Address review comments

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Further max Lut3D size cleanup

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Improve cast to avoid possible warning

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

---------

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

Author: doug-walker

SECURITY.md

@@ -14,20 +14,16 @@ would be naive to say our code is immune to every exploit.
 
 ## Reporting Vulnerabilities
 
-Quickly resolving security related issues is a priority. If you think
-you've found a potential vulnerability in OpenColorIO, please report it by
-emailing security@opencolorio.org. Only TSC members and ASWF project
-management have access to these messages.
+Quickly resolving security related issues is a priority. The best way to report a 
+vulnerability is to file a GitHub security advisory. If that is not possible, it 
+is also fine to email your report to security@opencolorio.org. Only the project 
+administrators have access to these reports.
 
 Include detailed steps to reproduce the issue, and any other information that
 could aid an investigation. Someone will assess the report and make every
 effort to respond within 14 days.
 
-## Outstanding Security Issues
-
-None
-
-## Addressed Security Issues
+## History of CVE Fixes
 
 None
 
@@ -64,6 +60,3 @@ set of behaviors as with file loading.
 It is a bug if calling a function with well-formed arguments causes the 
 library to crash. It is a security issue if calling a function with 
 well-formed arguments causes arbitrary code execution.
-
-We do not consider this as severe as file format issues because in most 
-deployments the parameter space is not exposed to potential attackers.

docs/site/homepage/config.toml

@@ -105,14 +105,6 @@ post_share = true
 enable = false
 preloader = "images/opencolorio-color.png"
 
-# google map
-[params.map]
-enable = false
-gmap_api = "https://maps.googleapis.com/maps/api/js?key=AIzaSyBu5nZKbeK-WHQ70oqOWo-_4VmwOwKP9YQ"
-map_latitude = "51.5223477"
-map_longitude = "-0.1622023"
-map_marker = "images/marker.png"
-
 
 ############################# ASWF LINKS ##########################
 [[params.aswf]]

ext/sampleicc/src/include/iccProfileReader.h

@@ -586,6 +586,10 @@ namespace SampleICC
             if (!Read32(istream, &sizeData, 1))
                 return false;
 
+            // ICC curve entries are indexed by 16-bit values; 65536 is the maximum.
+            if (sizeData > 65536)
+                return false;
+
             mCurve.resize(sizeData);
 
             if (sizeData)

src/OpenColorIO/ColorSpace.cpp

@@ -162,6 +162,7 @@ const char * ColorSpace::getAlias(size_t idx) const noexcept
 
 bool ColorSpace::hasAlias(const char * alias) const noexcept
 {
+    if (!alias) return false;
     for (size_t idx = 0; idx < getImpl()->m_aliases.size(); ++idx)
     {
         if (0 == Platform::Strcasecmp(getImpl()->m_aliases[idx].c_str(), alias))
@@ -430,6 +431,7 @@ int ColorSpace::getAllocationNumVars() const
 
 void ColorSpace::getAllocationVars(float * vars) const
 {
+    if(!vars) return;
     if(!getImpl()->m_allocationVars.empty())
     {
         memcpy(vars,
@@ -440,6 +442,15 @@ void ColorSpace::getAllocationVars(float * vars) const
 
 void ColorSpace::setAllocationVars(int numvars, const float * vars)
 {
+    if (numvars < 0)
+    {
+        throw Exception("setAllocationVars: numvars must not be negative.");
+    }
+    if (numvars > 0 && !vars)
+    {
+        throw Exception("setAllocationVars: vars must not be null when numvars is positive.");
+    }
+
     getImpl()->m_allocationVars.resize(numvars);
 
     if(!getImpl()->m_allocationVars.empty())

src/OpenColorIO/ColorSpaceSet.cpp

@@ -180,6 +180,7 @@ class ColorSpaceSet::Impl
 
     void remove(const char * csName)
     {
+        if (!csName || !*csName) return;
         const std::string name = StringUtils::Lower(csName);
         if (name.empty()) return;
 

src/OpenColorIO/Config.cpp

@@ -4337,7 +4337,7 @@ int Config::getNumDisplaysAll() const noexcept
 
 const char * Config::getDisplayAll(int index) const noexcept
 {
-    if (index >= 0 || index < static_cast<int>(getImpl()->m_displays.size()))
+    if (index >= 0 && index < static_cast<int>(getImpl()->m_displays.size()))
     {
         return getImpl()->m_displays[index].first.c_str();
     }
@@ -4365,7 +4365,7 @@ int Config::getDisplayAllByName(const char * name) const noexcept
 
 bool Config::isDisplayTemporary(int index) const noexcept
 {
-    if (index >= 0 || index < static_cast<int>(getImpl()->m_displays.size()))
+    if (index >= 0 && index < static_cast<int>(getImpl()->m_displays.size()))
     {
         return getImpl()->m_displays[index].second.m_temporary;
     }
@@ -4375,7 +4375,7 @@ bool Config::isDisplayTemporary(int index) const noexcept
 
 void Config::setDisplayTemporary(int index, bool isTemporary) noexcept
 {
-    if (index >= 0 || index < static_cast<int>(getImpl()->m_displays.size()))
+    if (index >= 0 && index < static_cast<int>(getImpl()->m_displays.size()))
     {
         getImpl()->m_displays[index].second.m_temporary = isTemporary;
 
@@ -4410,7 +4410,7 @@ const char * Config::getView(ViewType type, const char * display, int index) con
 {
     if (!display || !*display)
     {
-        if (index >= 0 || index < static_cast<int>(getImpl()->m_sharedViews.size()))
+        if (index >= 0 && index < static_cast<int>(getImpl()->m_sharedViews.size()))
         {
             return getImpl()->m_sharedViews[index].m_name.c_str();
         }
@@ -4449,11 +4449,19 @@ const char * Config::getView(ViewType type, const char * display, int index) con
 
 void Config::getDefaultLumaCoefs(double * c3) const
 {
+    if (!c3)
+    {
+        throw Exception("getDefaultLumaCoefs: c3 must not be null.");
+    }
     memcpy(c3, &getImpl()->m_defaultLumaCoefs[0], 3*sizeof(double));
 }
 
 void Config::setDefaultLumaCoefs(const double * c3)
 {
+    if (!c3)
+    {
+        throw Exception("setDefaultLumaCoefs: c3 must not be null.");
+    }
     memcpy(&getImpl()->m_defaultLumaCoefs[0], c3, 3*sizeof(double));
 
     AutoMutex lock(getImpl()->m_cacheidMutex);

src/OpenColorIO/ContextVariableUtils.cpp

@@ -101,10 +101,12 @@ void LoadEnvironment(EnvMap & map, bool update)
 
         const std::string env_str = (char*)*env;
 #endif
-        const int pos = static_cast<int>(env_str.find_first_of('='));
+        const auto pos = env_str.find_first_of('=');
+
+        if (pos == std::string::npos) continue;
 
         const std::string name  = env_str.substr(0, pos);
-        const std::string value = env_str.substr(pos+1, env_str.length());
+        const std::string value = env_str.substr(pos+1);
 
         if (update)
         {
@@ -122,8 +124,12 @@ void LoadEnvironment(EnvMap & map, bool update)
     }
 }
 
-std::string ResolveContextVariables(const std::string & str, const EnvMap & map, UsedEnvs & used)
+static std::string ResolveContextVariablesImpl(const std::string & str, const EnvMap & map,
+                                               UsedEnvs & used, int depth)
 {
+    // Guard against infinite recursion from cyclic variable references.
+    if (depth > 32) return str;
+
     // Early exit if no reserved tokens are found.
     if (!ContainsContextVariables(str))
     {
@@ -159,12 +165,17 @@ std::string ResolveContextVariables(const std::string & str, const EnvMap & map,
     // recursively call till string doesn't expand anymore
     if(newstr != orig)
     {
-        return ResolveContextVariables(newstr, map, used);
+        return ResolveContextVariablesImpl(newstr, map, used, depth + 1);
     }
 
     return orig;
 }
 
+std::string ResolveContextVariables(const std::string & str, const EnvMap & map, UsedEnvs & used)
+{
+    return ResolveContextVariablesImpl(str, map, used, 0);
+}
+
 bool CollectContextVariables(const Config & config, 
                              const Context & context,
                              ConstTransformRcPtr transform,

src/OpenColorIO/CustomKeys.h

@@ -61,14 +61,24 @@ class CustomKeysContainer
 
     bool hasKey(const char * key)
     {
-        std::string s = key;
-        return m_customKeys.count(s) > 0;
+        if (!key || !*key) return false;
+        return m_customKeys.count(key) > 0;
     }
 
     const char * getValueForKey(const char * key)
     {
-        // NB: Will throw if the map doesn't have the key.
-        return m_customKeys[key].c_str();
+        if (!key || !*key)
+        {
+            throw Exception("Key has to be a non-empty string.");
+        }
+        auto it = m_customKeys.find(key);
+        if (it == m_customKeys.end())
+        {
+            std::ostringstream oss;
+            oss << "Key '" << key << "' not found.";
+            throw Exception(oss.str().c_str());
+        }
+        return it->second.c_str();
     }
 
 private:

src/OpenColorIO/Display.cpp

@@ -52,7 +52,7 @@ void AddView(ViewVec & views, const char * name, const char * viewTransform,
              const char * displayColorSpace, const char * looks,
              const char * rule, const char * description)
 {
-    if (0 == Platform::Strcasecmp(displayColorSpace, OCIO_VIEW_USE_DISPLAY_NAME))
+    if (displayColorSpace && 0 == Platform::Strcasecmp(displayColorSpace, OCIO_VIEW_USE_DISPLAY_NAME))
     {
         displayColorSpace = OCIO_VIEW_USE_DISPLAY_NAME;
     }

src/OpenColorIO/Display.h

@@ -37,7 +37,7 @@ struct View
          const char * looks,
          const char * rule,
          const char * description)
-        : m_name(name)
+        : m_name(name ? name : "")
         , m_viewTransform(viewTransform ? viewTransform : "")
         , m_colorspace(colorspace ? colorspace : "")
         , m_looks(looks ? looks : "")