Adsk Contrib - Miscellaneous improvements suggested by Claude

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 : "")

src/OpenColorIO/DynamicProperty.cpp

@@ -280,6 +280,10 @@ void DynamicPropertyGradingRGBCurveImpl::precompute()
     {
         ConstGradingBSplineCurveRcPtr curve = m_gradingRGBCurve->getCurve(c);
         auto curveImpl = dynamic_cast<const GradingBSplineCurveImpl *>(curve.get());
+        if (!curveImpl)
+        {
+            throw Exception("DynamicPropertyGradingRGBCurveImpl: unexpected curve implementation.");
+        }
         curveImpl->computeKnotsAndCoefs(m_knotsCoefs, static_cast<int>(c), false);
     }
     if (m_knotsCoefs.m_numKnots <= 0) m_knotsCoefs.m_localBypass = true;
@@ -375,6 +379,10 @@ void DynamicPropertyGradingHueCurveImpl::precompute()
     {
         ConstGradingBSplineCurveRcPtr curve = m_gradingHueCurve->getCurve(c);
         auto curveImpl = dynamic_cast<const GradingBSplineCurveImpl *>(curve.get());
+        if (!curveImpl)
+        {
+            throw Exception("DynamicPropertyGradingHueCurveImpl: unexpected curve implementation.");
+        }
         curveImpl->computeKnotsAndCoefs(m_knotsCoefs, static_cast<int>(c),
                                         m_gradingHueCurve->getDrawCurveOnly());
     }

src/OpenColorIO/GpuShader.cpp

@@ -11,7 +11,7 @@
 
 #include "DynamicProperty.h"
 #include "GpuShader.h"
-#include "ops/lut3d/Lut3DOpData.h"
+#include "LutLimits.h"
 #include "Platform.h"
 
 namespace OCIO_NAMESPACE
@@ -194,7 +194,7 @@ class PrivateImpl
 
     virtual ~PrivateImpl() {}
 
-    inline unsigned get3dLutMaxLength() const { return Lut3DOpData::maxSupportedLength; }
+    inline unsigned get3dLutMaxLength() const { return Max3DLUTLength; }
 
     inline unsigned get1dLutMaxWidth() const { return m_max1DLUTWidth; }
     inline void set1dLutMaxWidth(unsigned maxWidth) { m_max1DLUTWidth = maxWidth; }

src/OpenColorIO/GpuShaderDesc.cpp

@@ -135,7 +135,7 @@ void GpuShaderCreator::setFunctionName(const char * name) noexcept
 {
     AutoMutex lock(getImpl()->m_cacheIDMutex);
     // Note: Remove potentially problematic double underscores from GLSL resource names.
-    getImpl()->m_functionName = StringUtils::Replace(name, "__", "_");
+    getImpl()->m_functionName = StringUtils::Replace(name ? name : "", "__", "_");
     getImpl()->m_cacheID.clear();
 }
 
@@ -148,7 +148,7 @@ void GpuShaderCreator::setResourcePrefix(const char * prefix) noexcept
 {
     AutoMutex lock(getImpl()->m_cacheIDMutex);
     // Note: Remove potentially problematic double underscores from GLSL resource names.
-    getImpl()->m_resourcePrefix = StringUtils::Replace(prefix, "__", "_");
+    getImpl()->m_resourcePrefix = StringUtils::Replace(prefix ? prefix : "", "__", "_");
     getImpl()->m_cacheID.clear();
 }
 
@@ -161,7 +161,7 @@ void GpuShaderCreator::setPixelName(const char * name) noexcept
 {
     AutoMutex lock(getImpl()->m_cacheIDMutex);
     // Note: Remove potentially problematic double underscores from GLSL resource names.
-    getImpl()->m_pixelName = StringUtils::Replace(name, "__", "_");
+    getImpl()->m_pixelName = StringUtils::Replace(name ? name : "", "__", "_");
     getImpl()->m_cacheID.clear();
 }
 

src/OpenColorIO/HashUtils.cpp

@@ -22,7 +22,9 @@ std::string CacheIDHash(const char * array, std::size_t size)
     XXH128_hash_t hash = XXH3_128bits(array, size);
 
     std::stringstream oss;
-    oss << std::hex << hash.low64 << hash.high64;
+    oss << std::hex << std::setfill('0');
+    oss << std::setw(16) << hash.low64;
+    oss << std::setw(16) << hash.high64;
     return oss.str();
 }
 

src/OpenColorIO/ImageDesc.cpp

@@ -260,6 +260,7 @@ struct PackedImageDesc::Impl
         {
             // Confirm xStrideBytes is a pure packing
             // (I.e., it will divide evenly)
+            if (m_chanStrideBytes == 0) return false;
             const div_t result = div((int)m_xStrideBytes, (int)m_chanStrideBytes);
             if(result.rem != 0) return false;
 

src/OpenColorIO/Logging.cpp

@@ -114,11 +114,17 @@ void SetLoggingLevel(LoggingLevel level)
 
 void SetLoggingFunction(LoggingFunction logFunction)
 {
+    if (!logFunction)
+    {
+        throw Exception("SetLoggingFunction: logFunction must not be null.");
+    }
+    AutoMutex lock(g_logmutex);
     g_loggingFunction = logFunction;
 }
 
 void ResetToDefaultLoggingFunction()
 {
+    AutoMutex lock(g_logmutex);
     g_loggingFunction = DefaultLoggingFunction;
 }
 

src/OpenColorIO/LutLimits.h

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: BSD-3-Clause
+// Copyright Contributors to the OpenColorIO Project.
+
+#ifndef INCLUDED_OCIO_LUTLIMITS_H
+#define INCLUDED_OCIO_LUTLIMITS_H
+
+namespace OCIO_NAMESPACE
+{
+
+// Maximum number of entries supported in a 1D LUT.
+constexpr unsigned long Max1DLUTLength = 300000;
+
+// Maximum grid size supported for a 3D LUT.
+// 129 allows for a MESH dimension of 7 in the 3dl file format.
+constexpr unsigned long Max3DLUTLength = 129;
+
+} // namespace OCIO_NAMESPACE
+
+#endif // INCLUDED_OCIO_LUTLIMITS_H

src/OpenColorIO/NamedTransform.cpp

@@ -67,6 +67,7 @@ const char * NamedTransformImpl::getAlias(size_t idx) const noexcept
 
 bool NamedTransformImpl::hasAlias(const char * alias) const noexcept
 {
+    if (!alias || !*alias) return false;
     for (size_t idx = 0; idx < m_aliases.size(); ++idx)
     {
         if (0 == Platform::Strcasecmp(m_aliases[idx].c_str(), alias))