GH-50522: [C++][Gandiva] fix heap overflow in aes_encrypt output sizing

[Arawoof06]

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.

Was AI used for this PR?

In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.

PR code and description written by:

• Human
• AI

AI tooling was used to find the bug and to write the fix, the regression test and this description. The regression test was run against the unpatched code, where it fails, and against the patched code before submission.

Reviewed before submission by:

• Human
• AI
• Not reviewed

• GitHub Issue: [C++][Gandiva] Heap buffer overflow in aes_encrypt output buffer sizing #50522

[github-actions]

⚠️ GitHub issue #50522 has been automatically assigned in GitHub to PR creator.

[github-actions]

⚠️ GitHub issue #50522 has no components, please add labels for components.

[Arawoof06]

any update here?

[Arawoof06]

Rebased onto main to get a fresh CI run. The only red before was the Windows MSVC GLib job, which died on an SSL error pulling CMake during vcpkg install (that job builds with Gandiva off, so unrelated to this change). New run is going now.

[github-actions]

⚠️ GitHub issue #50522 has no components, please add labels for components.

[dmitry-chirkov-dremio]

Issue says a 24-byte key can round to a value "below data_len". Could you confirm if it does?
It appears to be rounding below the needed ciphertext length, which is a different statement.

I'd say rebase, get a fresh build and ping me for review.

[Arawoof06]

You're right, and the issue wording is off. With a 24-byte key RoundUpToPowerOf2(data_len, 24) never lands below data_len. 24 isn't a power of two so the mask isn't clean, but it still rounds to >= data_len for every length I checked. The shortfall is against the ciphertext length instead: data_len=8 rounds to 8 while the ciphertext is 16, and data_len=32 rounds to 32 while the ciphertext is 48. So it comes up short of the needed output, not below data_len. The real cause either way is the missing padding block, which is what the fix sizes in.

Rebased onto main and force-pushed, so fresh CI is running. Ready for your review.

[dmitry-chirkov-dremio]

You're right, and the issue wording is off.

Could you update the issue's text to reflect that, please?

[Arawoof06]

Updated the issue. It now says the 24-byte key rounding never goes below data_len and that the shortfall is against the ciphertext length (data_len 8 rounds to 8, ciphertext is 16).

[dmitry-chirkov-dremio]

The unchecked narrowing (here and below) predates this PR, but adding the padding block makes another 16 input lengths overflow. Could we guard the new allocation calculation before narrowing, in both stubs?

[dmitry-chirkov-dremio]

I'd be ok with a pushback and tracking this as a separate issue.

[Arawoof06]

Makes sense to fix it here since the padding block is what adds those lengths. Both stubs now compute the size as int64 and set an error before the narrowing if it's above INT32_MAX. Added TestAesOutputSizeOverflow, which covers INT32_MAX - 15 and INT32_MAX for encrypt and decrypt.