From 6bb87781623ec202bc9a7994893bf863424ca712 Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Sun, 24 May 2026 19:10:26 -0400 Subject: [PATCH 1/2] Fix potential cycle crash Signed-off-by: Doug Walker --- src/OpenColorIO/ContextVariableUtils.cpp | 17 +++- src/OpenColorIO/Transform.cpp | 15 +++ tests/cpu/Config_tests.cpp | 121 +++++++++++++++++++++++ tests/python/ConfigTest.py | 21 ++++ 4 files changed, 173 insertions(+), 1 deletion(-) diff --git a/src/OpenColorIO/ContextVariableUtils.cpp b/src/OpenColorIO/ContextVariableUtils.cpp index 82726d4234..1cd61c811f 100644 --- a/src/OpenColorIO/ContextVariableUtils.cpp +++ b/src/OpenColorIO/ContextVariableUtils.cpp @@ -176,11 +176,26 @@ std::string ResolveContextVariables(const std::string & str, const EnvMap & map, return ResolveContextVariablesImpl(str, map, used, 0); } -bool CollectContextVariables(const Config & config, +bool CollectContextVariables(const Config & config, const Context & context, ConstTransformRcPtr transform, ContextRcPtr & usedContextVars) { + // Guard against infinite recursion from cyclic color space / look / view transform + // references in malicious configs (e.g. a ColorSpace whose from_reference is a + // ColorSpaceTransform pointing back to itself). + static thread_local int depth = 0; + if (depth > 32) + { + throw Exception("Cycle detected while collecting context variables."); + } + struct DepthGuard + { + int & d; + DepthGuard(int & d_) : d(d_) { ++d; } + ~DepthGuard() { --d; } + } guard(depth); + if(ConstColorSpaceTransformRcPtr tr = DynamicPtrCast(transform)) { if (CollectContextVariables(config, context, *tr, usedContextVars)) return true; diff --git a/src/OpenColorIO/Transform.cpp b/src/OpenColorIO/Transform.cpp index 8b2b880699..49ed7ce97b 100755 --- a/src/OpenColorIO/Transform.cpp +++ b/src/OpenColorIO/Transform.cpp @@ -49,6 +49,21 @@ void BuildOps(OpRcPtrVec & ops, if(!transform) return; + // Guard against infinite recursion from cyclic color space / look / view transform + // references in malicious configs (e.g. a ColorSpace whose from_reference is a + // ColorSpaceTransform pointing back to itself). + static thread_local int depth = 0; + if (depth > 32) + { + throw Exception("Cycle detected while building ops from transform."); + } + struct DepthGuard + { + int & d; + DepthGuard(int & d_) : d(d_) { ++d; } + ~DepthGuard() { --d; } + } guard(depth); + if(ConstAllocationTransformRcPtr allocationTransform = \ DynamicPtrCast(transform)) { diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index b3d69c1688..1cbf807721 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -10416,3 +10416,124 @@ OCIO_ADD_TEST(Config, interchange_attributes) } } +OCIO_ADD_TEST(Config, cyclic_color_space_transform) +{ + // Regression test for a stack overflow triggered by a self-referential + // ColorSpaceTransform (a ColorSpace whose from_reference is a ColorSpaceTransform + // that points back to itself). getProcessor() must throw a clean exception + // rather than recursing until the stack guard page is hit. + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " isdata: true\n" + " - !\n" + " name: cs1\n" + " from_scene_reference: ! {src: cs0, dst: cs1}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->getProcessor("cs0", "cs1"), + OCIO::Exception, + "Cycle detected"); +} + +OCIO_ADD_TEST(Config, cyclic_color_space_two_step) +{ + // Two-step cycle: cs1 -> cs2 -> cs1. Verify the depth guard catches indirect cycles too. + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " isdata: true\n" + " - !\n" + " name: cs1\n" + " from_scene_reference: ! {src: cs0, dst: cs2}\n" + " - !\n" + " name: cs2\n" + " from_scene_reference: ! {src: cs0, dst: cs1}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->getProcessor("cs0", "cs1"), + OCIO::Exception, + "Cycle detected"); +} + +OCIO_ADD_TEST(Config, cyclic_look_transform) +{ + // Regression test: a Look whose transform is a LookTransform that references the + // same look must not crash via stack overflow on getProcessor(). Triggered here + // by a ColorSpace whose from_scene_reference is a LookTransform invoking lookA, + // whose own transform is another LookTransform invoking lookA. + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "looks:\n" + " - !\n" + " name: lookA\n" + " process_space: cs0\n" + " transform: ! {src: cs0, dst: cs0, looks: lookA}\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " - !\n" + " name: cs1\n" + " from_scene_reference: ! {src: cs0, dst: cs0, looks: lookA}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->getProcessor("cs0", "cs1"), + OCIO::Exception, + "Cycle detected"); +} + +OCIO_ADD_TEST(Config, cyclic_display_view_transform) +{ + // Regression test: a ColorSpace whose from_scene_reference is a DisplayViewTransform + // that ultimately points back to the same ColorSpace (via the view's color space) + // must not crash via stack overflow on getProcessor(). + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "displays:\n" + " D1:\n" + " - ! {name: V1, colorspace: cs_loop}\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " - !\n" + " name: cs_loop\n" + " from_scene_reference: ! {src: cs0, display: D1, view: V1}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->getProcessor("cs0", "cs_loop"), + OCIO::Exception, + "Cycle detected"); +} + diff --git a/tests/python/ConfigTest.py b/tests/python/ConfigTest.py index 9debc3da2e..6f3baff7c3 100644 --- a/tests/python/ConfigTest.py +++ b/tests/python/ConfigTest.py @@ -1446,6 +1446,27 @@ def test_active__displayview_lists(self): config.addActiveView(view="v1") self.assertEqual(config.getNumActiveViews(), 1) + def test_cyclic_color_space_transform(self): + """ + Regression test: a ColorSpace whose from_scene_reference is a + ColorSpaceTransform pointing back to itself must not crash the + process via stack overflow on getProcessor(). + """ + CYCLIC_PROFILE = """ocio_profile_version: 2 +roles: + default: cs0 +colorspaces: + - ! + name: cs0 + isdata: true + - ! + name: cs1 + from_scene_reference: ! {src: cs0, dst: cs1} +""" + cfg = OCIO.Config.CreateFromStream(CYCLIC_PROFILE) + with self.assertRaises(OCIO.Exception): + cfg.getProcessor("cs0", "cs1") + class ConfigVirtualWithActiveDisplayTest(unittest.TestCase): def setUp(self): From 85f67ccc44390ed94e540f2566cadd8a682aacbc Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Sun, 24 May 2026 20:33:20 -0400 Subject: [PATCH 2/2] Clarify purpose of the various guards Signed-off-by: Doug Walker --- src/OpenColorIO/ContextVariableUtils.cpp | 15 ++++++++---- src/OpenColorIO/Transform.cpp | 12 +++++++--- tests/cpu/Config_tests.cpp | 29 ++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/OpenColorIO/ContextVariableUtils.cpp b/src/OpenColorIO/ContextVariableUtils.cpp index 1cd61c811f..8340a5cd30 100644 --- a/src/OpenColorIO/ContextVariableUtils.cpp +++ b/src/OpenColorIO/ContextVariableUtils.cpp @@ -127,7 +127,10 @@ void LoadEnvironment(EnvMap & map, bool update) static std::string ResolveContextVariablesImpl(const std::string & str, const EnvMap & map, UsedEnvs & used, int depth) { - // Guard against infinite recursion from cyclic variable references. + // Guard against infinite recursion at the string substitution level (e.g. $A expands + // to $B which expands to $A). This is independent from the graph-level guards in + // CollectContextVariables() and Transform.cpp's BuildOps(), which protect against cycles + // in the color space / look / view transform reference graph. if (depth > 32) return str; // Early exit if no reserved tokens are found. @@ -181,9 +184,13 @@ bool CollectContextVariables(const Config & config, ConstTransformRcPtr transform, ContextRcPtr & usedContextVars) { - // Guard against infinite recursion from cyclic color space / look / view transform - // references in malicious configs (e.g. a ColorSpace whose from_reference is a - // ColorSpaceTransform pointing back to itself). + // Guard against infinite recursion through cycles in the color space / look / view + // transform reference graph (e.g. a ColorSpace whose from_reference is a + // ColorSpaceTransform pointing back to itself). Distinct from the string-level depth + // check in ResolveContextVariablesImpl, which only guards cyclic context-variable + // substitution within a single string; that check fires once per finite resolveStringVar + // call here and never observes this outer graph traversal. A matching guard exists in + // BuildOps() for the op-builder traversal that runs after this one. static thread_local int depth = 0; if (depth > 32) { diff --git a/src/OpenColorIO/Transform.cpp b/src/OpenColorIO/Transform.cpp index 49ed7ce97b..061f1f733b 100755 --- a/src/OpenColorIO/Transform.cpp +++ b/src/OpenColorIO/Transform.cpp @@ -49,9 +49,15 @@ void BuildOps(OpRcPtrVec & ops, if(!transform) return; - // Guard against infinite recursion from cyclic color space / look / view transform - // references in malicious configs (e.g. a ColorSpace whose from_reference is a - // ColorSpaceTransform pointing back to itself). + // Guard against infinite recursion through cycles in the color space / look / view + // transform reference graph (e.g. a ColorSpace whose from_reference is a + // ColorSpaceTransform pointing back to itself). A matching guard in + // CollectContextVariables() catches the same cycles during the earlier context-variable + // scan; this one protects the op-builder traversal. Both are independent from the + // string-level depth check in ResolveContextVariablesImpl, which guards cyclic + // context-variable substitution within a single string. Having the extra guard here + // is necessary for any situations where CollectContextVariables is not called, for + // example, see Config_tests.cpp/cyclic_color_space_linearity_check. static thread_local int depth = 0; if (depth > 32) { diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 1cbf807721..28d5e430b1 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -10507,6 +10507,35 @@ OCIO_ADD_TEST(Config, cyclic_look_transform) "Cycle detected"); } +OCIO_ADD_TEST(Config, cyclic_color_space_linearity_check) +{ + // Regression test for the BuildOps depth guard. Config::isColorSpaceLinear() reaches + // the op-builder via Config::Impl::getProcessorWithoutCaching(), which does NOT call + // CollectContextVariables() first. So even with the CollectContextVariables guard, + // this path will still infinite-recurse on a cyclic color space unless BuildOps + // independently guards against it. + + constexpr char CYCLIC_PROFILE[] = + "ocio_profile_version: 2\n" + "roles:\n" + " default: cs0\n" + "colorspaces:\n" + " - !\n" + " name: cs0\n" + " - !\n" + " name: cs1\n" + " from_scene_reference: ! {src: cs0, dst: cs1}\n"; + + std::istringstream is; + is.str(CYCLIC_PROFILE); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + + OCIO_CHECK_THROW_WHAT(config->isColorSpaceLinear("cs1", OCIO::REFERENCE_SPACE_SCENE), + OCIO::Exception, + "Cycle detected"); +} + OCIO_ADD_TEST(Config, cyclic_display_view_transform) { // Regression test: a ColorSpace whose from_scene_reference is a DisplayViewTransform