fix: add ShowCard nesting depth limit to prevent stack overflow#9371
Open
karan68 wants to merge 1 commit intomicrosoft:mainfrom
Open
fix: add ShowCard nesting depth limit to prevent stack overflow#9371karan68 wants to merge 1 commit intomicrosoft:mainfrom
karan68 wants to merge 1 commit intomicrosoft:mainfrom
Conversation
ShowCardActionParser::Deserialize calls AdaptiveCard::Deserialize recursively with no depth guard, allowing a malicious card with deeply nested ShowCards to cause a stack overflow crash in the XAML renderer (BuildXamlTreeFromAdaptiveCard -> BuildActions -> BuildInlineShowCard -> BuildShowCard -> recurse). This adds a depth counter to ParseContext that caps ShowCard nesting at 5 levels. ShowCards beyond the limit get an empty inner card and a parse warning is emitted. Verified: depth 200 previously created a fully-recursive tree of 200 AdaptiveCard objects; now capped at 5 with warning. Security: Fixes unbounded recursion that can crash WidgetBoard.exe Related: microsoft#9343
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a maximum nesting depth limit (5 levels) for Action.ShowCard in the shared C++ object model parser. This prevents deeply nested ShowCard payloads from causing a stack overflow crash in downstream renderers.
Problem
ShowCardActionParser::Deserialize callsAdaptiveCard::Deserialize recursively with zero depth tracking. A malicious or malformed card payload with deeply nested Action.ShowCard actions creates an unbounded recursive parse tree. When this tree is then rendered by the XAML renderer (BuildXamlTreeFromAdaptiveCard → BuildActions → BuildInlineShowCard → BuildShowCard → recurse), the deep recursion overflows the stack, crashing the host process.Verified locally: A card with 325 levels of ShowCard nesting parses successfully into a fully-recursive tree of 325 AdaptiveCard objects — the parser provides no resistance whatsoever. The only incidental barrier is jsoncpp's stackLimit (1000 JSON nesting levels), which is not an AdaptiveCards security control.This is relevant to all renderers that use the shared C++ model (UWP, WinUI3, Android, iOS), since the unbounded tree is constructed at parse time before any renderer-specific code runs.
Changes
4 files changed, 146 insertions:
c_maxShowCardDepth(5), depth counter, andCanIncrementShowCardDepth()/IncrementShowCardDepth()/DecrementShowCardDepth()methodsAdaptiveCard::Deserializecall inShowCardActionParser::Deserializewhen depth exceeds the limit, sets an empty inner card and emits aCustomWarningHow it works
Behavior
No breaking changes. Real-world cards never nest ShowCards deeper than 2–3 levels. The limit of 5 is generous.
Testing
Verified with a standalone C++ test binary that builds against the ObjectModel library:
4 unit tests also added to
ObjectModelTest.cppin the existing CppUnitTest project.