Skip to content

Fix consClose corrupting decoder state for large enums #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Unisay wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix consClose corrupting decoder state for large enums #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions src/Flat/Decoder/Prim.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,10 @@ consOpen = Get $ \endPtr s -> do
else notEnoughSpace endPtr s
return $ GetResult s (ConsState w 0)

-- | Switch back to normal decoding
-- {-# NOINLINE consClose #-}
-- | Switch back to normal decoding by advancing the main stream
-- past the constructor tag bits that were decoded via 'ConsState'.
consClose :: Int -> Get ()
consClose n = Get $ \endPtr s -> do
let u' = n + usedBits s
if u' < 8
then return $ GetResult (s {usedBits = u'}) ()
else
if currPtr s >= endPtr
then notEnoughSpace endPtr s
else return $ GetResult (s {currPtr = currPtr s `plusPtr` 1, usedBits = u' - 8}) ()
consClose = dropBits

{- ensureBits endPtr s n = when ((endPtr `minusPtr` currPtr s) * 8 - usedBits s < n) $ notEnoughSpace endPtr s
dropBits8 s n =
Expand Down
28 changes: 28 additions & 0 deletions test/Spec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,34 @@ testLargeEnum = testGroup "test enum with more than 256 constructors"
-- , encRaw (E258_256,E258_257,E258_258) [0b11111110,0b11111111,0b01111111,0b11000000]
, map trip [E258_1, E258_256, E258_257, E258_258]
, map trip [E256_1, E256_134, E256_256]
-- Issue #7542: consClose only advances currPtr by 1 byte, so when
-- (constructor_bits + usedBits >= 16) the decoder state is corrupted
-- (usedBits overflows to 8+). This makes the Filler decoder loop
-- forever, building infinite FillerBit chains and consuming all memory.
--
-- The bug requires: (a) a constructor needing 9 bits (depth-9 in the
-- Generic tree), AND (b) 7 prior consumed bits so usedBits=7 before
-- consOpen. We use 7 nested Bool fields to set up condition (b).
--
-- Control: E258_256 needs only 8 bits, so 8+7=15 < 16 - no overflow.
, [trip (False, (False, (False, (False, (False, (False, (False, E258_256)))))))]
-- Bug: E258_258 needs 9 bits, so 9+7=16 - consClose overflows usedBits.
-- Without fix: unflat hangs forever (Filler decoder infinite loop).
, [localOption (mkTimeout 5000000) $
trip (False, (False, (False, (False, (False, (False, (False, E258_258)))))))]
-- consClose must reject when constructor bits exceed the remaining buffer,
-- not silently leave the decoder in an invalid state for strictDecoder to
-- catch later. E258_258 needs 9 bits but a 1-byte buffer only has 8.
-- With correct bounds checking (ensureBits), consClose throws NotEnoughSpace.
-- Without it, consClose "succeeds" and strictDecoder catches TooMuchSpace.
, [testCase "consClose rejects 9-bit tag in 1-byte buffer" $
case unflatRaw (B.pack [0xFF]) :: Decoded E258 of
Left (NotEnoughSpace _) -> return ()
Left (TooMuchSpace _) -> assertFailure
"consClose let overrun through (caught by strictDecoder as TooMuchSpace)"
Left other -> assertFailure $ "Unexpected error: " ++ show other
Right _ -> assertFailure "Should not decode: only 8 bits for a 9-bit tag"
]
#endif
]

Expand Down