Repository navigation
Bound memory and stack use when decoding and parsing - #2813
Conversation
d11d580 to
6ae3f92
Compare
There was a problem hiding this comment.
For this change there's quite a lot of recursive processing of types in WIT and component-model types in general. This is why the validation of a component limits the depth of types to a fixed amount to ensure that all code downstream which processes types recursively is safe to do so. My guess is that this is not the only location throughout wit-parser that uses recursion and doesn't handle deeply-nested types well, and I don't think it's necessarily worthwhile to try to patch every single one of them into a stack.
Alternatively, I would personally be more partial to an approach like validation does, which is to basically just validate that types aren't too deep. For example when creating a Resolve or similar that could verify type depth pretty quickly (in O(types) time). The validation would match what's already a restriction for components anyway, and it would mean that each individual location of recursion throughout the wit-parser crate doesn't have to get updated.
There was a problem hiding this comment.
I see the usual limits is 100. Going with that.
Decoding read each section into a buffer sized up front by the length the input gives for it. A 14-byte input claiming a 4 GiB section made it allocate 4 GiB, which aborts the process where that fails. The buffer now grows with what is actually read.
WIT already rejected nesting 100 types deep within one type, such as
`list<list<...>>`, but not through named types and aliases. So
`record r1 { x: r0 }` repeated 200,000 times parsed, and then overflowed
the stack in code that recurses through types, such as
`component embed --dummy`.
The same limit now applies once a package is resolved, counting named
types and aliases.
6ae3f92 to
ad8073d
Compare
There was a problem hiding this comment.
Not sure how necessary these are per your standards. But keeping as a guard that may be flagged later if the encoder behaviour changes and loop fails.
Part 1/x of a series of PRs backporting fixes and workarounds discovered via fuzzing
For inputs that may abort the process instead of erroring: a section length that made decoding allocate what it claims (4 GiB from a 14-byte input; might be slightly slower due to resizes but safer imo), and a long chain of type aliases that overflowed the stack while parsing WIT.