Arawoof06 opened a new pull request, #50523: URL: https://github.com/apache/arrow/pull/50523
### Rationale for this change `gdv_fn_aes_encrypt` sizes its output buffer with `RoundUpToPowerOf2(data_len, key_data_len)`, which treats the key length as the AES block size. AES works on 16 byte blocks for every supported key length, so the rounding factor is wrong, and a 24 byte key asks `RoundUpToPowerOf2` to round to a factor that is not a power of two, which its contract requires. On top of that, PKCS#7 padding always appends a whole block, so a block aligned input produces a ciphertext one block longer than the input and the buffer comes up short. Allocated size against the ciphertext OpenSSL actually writes: | key | data_len | allocated | actual ciphertext | |-----|----------|-----------|-------------------| | 16 | 16 | 16 | 32 | | 16 | 8192 | 8192 | 8208 | | 24 | 8 | 8 | 16 | | 32 | 32 | 32 | 48 | `aes_encrypt(col, key)` over any 16/32/48/... byte value lets `EVP_EncryptFinal_ex` write 16 bytes past the arena buffer, and a 24 byte key with an 8 byte value writes 8 past. The overrun lands on whatever the arena hands out next, so the neighbouring row's output is corrupted; once the value exceeds the arena chunk size the chunk is allocated exactly and the write runs off the end of it. `gdv_fn_aes_decrypt` sizes its buffer with the same expression. Its plaintext is never longer than the ciphertext so it does not overrun today, but the size it computes can equal `data_len` while OpenSSL documents `inl + block_size` as the output space `EVP_DecryptUpdate` may use, and the trailing `ret[*out_len] = '\0'` needs the room as well. ### What changes are included in this PR? Both stubs now size the output from the fixed 16 byte AES block plus one block for padding, which covers the exact PKCS#7 ciphertext length and the output space OpenSSL asks for. The key length keeps its 16/24/32 validation and only picks the cipher. ### Are these changes tested? Yes. `TestGdvFnStubs.TestAesEncryptBlockAlignedInput` encrypts two block aligned values from one context and decrypts the first. The undersized buffer makes the second ciphertext land on top of the first, so the decrypt fails on the current code and round-trips after the fix. It needs no sanitizer, which the existing Gandiva CI jobs do not enable. Full `gandiva-internals-test`, `gandiva-precompiled-test` and `gandiva-projector-test` pass. ### Are there any user-facing changes? No. **This PR contains a "Critical Fix".** It fixes a heap buffer overflow in `aes_encrypt` that corrupts neighbouring output (or writes off the end of the arena chunk) for any input whose length is a multiple of the AES block size. * GitHub Issue: #50522 -- 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]
