tbuktu / libntru

C Implementation of NTRUEncrypt
Other
202 stars 59 forks source link

Buffer overrun in ntru_to_arr_32 and ntru_to_arr_64 #24

Closed jquesnelle closed 9 years ago

jquesnelle commented 9 years ago

There is a buffer overrun in the ntru_to_arr_32() and ntru_to_arr_64() functions in poly.c. Both functions operate on the input buffer in 4-byte or 8-byte increments, but the buffer may not be a multiple of this length. For example, in test_arr() function we allocate a buffer 'a' of length 1495 and pass it to ntru_to_arr_32(), resulting in a write of 1496 bytes to this 1495 buffer.

Interestingly enough, the documentation for ntru_to_arr_32() explicitly claims that the buffer does NOT need any padding: "No extra room is needed at the end." This problem is somewhat mitigated on SSE3 builds since ntru_to_arr() which most of the library uses will call the SSE3 version, but the tests and at least ntru_export_pub() directly rely on the broken version. The SSE3 version of the function (ntru_to_arr_sse_2048()) does not have the problem (as far as I can tell).

FYI, I'm building in Microsoft Visual Studio which doesn't support C99's variable length arrays (I also have SSE3 turned off), so I changed all VLA allocations to _alloca() calls. In debug mode Visual Studio will warn of overruns on memory allocated by _alloca(), which is how I found this.

tbuktu commented 9 years ago

Sorry I didn't get around to this until now. Thank you for your fix.

I'd be interested in merging your changes for Visual Studio. Do you want to share them?

jquesnelle commented 9 years ago

I will clean it up and report back -- right now it's a mess of #ifdefs :D