zeroshade commented on PR #1012: URL: https://github.com/apache/arrow-go/pull/1012#issuecomment-5094392836
This has gone stale — #1027 landed on `main` a short while ago and implements the same physical-type-width delta wraparound as the encoder half of this PR, so GitHub now reports this as conflicting. The header-validation half is still valuable and I would like to take it. On `main` the decoder accepts `blockSize == 0` and an unbounded `miniBlocksPerBlock`, and the `deltaBitWidths.Resize(int(d.miniBlocksPerBlock))` on an attacker-controlled count is a real allocation hazard on untrusted files; the block-size/miniblock divisibility and INT32 range checks are all correct and I could not construct a valid input they reject. Could you rebase onto `main` and drop whatever the #1027 change already covers, keeping the `SetData`/`initBlock`/`unpackNextMini` validation and the used-bit-width check? One thing worth calling out in the description when you do: rejecting used bit widths above 32 for INT32 means files written by older arrow-go versions that emitted pathological 33-bit miniblocks will now fail to read. I believe that is correct, since those files are non-conformant and other Parquet implementations reject them too, but it should be explicit. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
