commit 9c2c6351db480d0be383168cc9cf9c908b9809e7
parent b57977b9fb74b61f4b5a26847732af23fe6f2268
Author: yao-msft <50888816+yao-msft@users.noreply.github.com>
Date: Wed, 5 Mar 2025 14:06:21 -0800
Add max nest level limit to yaml parser (#5275)
This fixes an oom issue found in fuzzing run.
Diffstat:
5 files changed, 131 insertions(+), 0 deletions(-)
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
@@ -1040,6 +1040,9 @@
<CopyFileToFolders Include="TestData\ContainsEscapeControlCode.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
+ <CopyFileToFolders Include="TestData\ContainsTooManyNestedLayers.yaml">
+ <DeploymentContent>true</DeploymentContent>
+ </CopyFileToFolders>
<CopyFileToFolders Include="TestData\ManifestV1_10-Bad-SchemaHeaderInvalid.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
@@ -1053,6 +1053,9 @@
<CopyFileToFolders Include="TestData\ContainsEscapeControlCode.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
+ <CopyFileToFolders Include="TestData\ContainsTooManyNestedLayers.yaml">
+ <Filter>TestData</Filter>
+ </CopyFileToFolders>
<CopyFileToFolders Include="TestData\ManifestV1_10-Bad-SchemaHeaderInvalid.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
diff --git a/src/AppInstallerCLITests/TestData/ContainsTooManyNestedLayers.yaml b/src/AppInstallerCLITests/TestData/ContainsTooManyNestedLayers.yaml
@@ -0,0 +1,101 @@
+-
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ -
+ - value
diff --git a/src/AppInstallerCLITests/Yaml.cpp b/src/AppInstallerCLITests/Yaml.cpp
@@ -173,3 +173,8 @@ TEST_CASE("YamlContainsEscapeControlCode", "[YAML]")
{
REQUIRE_THROWS_HR(Load(TestDataFile("ContainsEscapeControlCode.yaml")), APPINSTALLER_CLI_ERROR_LIBYAML_ERROR);
}
+
+TEST_CASE("YamlContainsTooManyNestedLayers", "[YAML]")
+{
+ REQUIRE_THROWS_HR(Load(TestDataFile("ContainsTooManyNestedLayers.yaml")), APPINSTALLER_CLI_ERROR_YAML_DOC_BUILD_FAILED);
+}
diff --git a/src/AppInstallerSharedLib/YamlWrapper.cpp b/src/AppInstallerSharedLib/YamlWrapper.cpp
@@ -151,6 +151,9 @@ namespace AppInstaller::YAML::Wrapper
size_t childOffset = 0;
};
+ static int YAML_DOCUMENT_NEST_LEVEL_LIMIT = 100;
+ int nestLevel = 0;
+
std::stack<StackItem> resultStack;
resultStack.emplace(root, &result);
@@ -173,6 +176,12 @@ namespace AppInstaller::YAML::Wrapper
break;
case YAML_SEQUENCE_NODE:
{
+ if (stackItem.childOffset == 0)
+ {
+ // We've entered the sequence.
+ nestLevel++;
+ }
+
yaml_node_item_t* child = stackItem.yamlNode->data.sequence.items.start + stackItem.childOffset++;
if (child < stackItem.yamlNode->data.sequence.items.top)
{
@@ -184,11 +193,18 @@ namespace AppInstaller::YAML::Wrapper
{
// We've reached the end of the sequence
pop = true;
+ nestLevel--;
}
break;
}
case YAML_MAPPING_NODE:
{
+ if (stackItem.childOffset == 0)
+ {
+ // We've entered the mapping.
+ nestLevel++;
+ }
+
yaml_node_pair_t* child = stackItem.yamlNode->data.mapping.pairs.start + stackItem.childOffset++;
if (child < stackItem.yamlNode->data.mapping.pairs.top)
{
@@ -207,6 +223,7 @@ namespace AppInstaller::YAML::Wrapper
{
// We've reached the end of the mapping
pop = true;
+ nestLevel--;
}
break;
}
@@ -216,6 +233,8 @@ namespace AppInstaller::YAML::Wrapper
{
resultStack.pop();
}
+
+ THROW_HR_IF_MSG(APPINSTALLER_CLI_ERROR_YAML_DOC_BUILD_FAILED, nestLevel > YAML_DOCUMENT_NEST_LEVEL_LIMIT, "Too many layers of nested nodes.");
}
return result;
