Skip to content
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.

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

Undefined behavior in AlignmentBuffer::fill_up_with_zeros #3734

Closed
guidovranken opened this issue Oct 6, 2023 · 5 comments · Fixed by #3736
Closed

Undefined behavior in AlignmentBuffer::fill_up_with_zeros #3734

guidovranken opened this issue Oct 6, 2023 · 5 comments · Fixed by #3736
Assignees
Labels
Milestone

Comments

@guidovranken
Copy link

guidovranken commented Oct 6, 2023

Set CXXFLAGS to -D_GLIBCXX_DEBUG (if you're using the GNU libc) or to -D_LIBCPP_DEBUG=1 (if you're using the LLVM libc). Or set both if you're unsure. Then compile Botan and the reproducer below.

#include <botan/kdf.h>

int main(void)
{
    const std::vector<uint8_t> password{0xc0, 0x99, 0xf7, 0x37, 0x45, 0x13, 0xe6, 0xc8, 0xd6, 0xa8};
    const std::vector<uint8_t> salt{0xb4, 0xad, 0xdb, 0x86, 0x49, 0x8b, 0xb9, 0xcc, 0x8a, 0x02, 0xd5, 0xa0, 0x60, 0xc0, 0x97, 0x30,
        0xa8, 0x6a, 0x66, 0x98, 0x52, 0x9f, 0xa4, 0x04, 0x41, 0x0a, 0xbd, 0x9e, 0x59, 0x55, 0x0f, 0x82,
        0x7f, 0x1f, 0x06, 0xda, 0xa0, 0x0f, 0x03, 0xcc, 0x97, 0xcf, 0x71, 0x82, 0x54, 0x00, 0xb7, 0x1e,
        0xbc, 0x50, 0x34, 0xc3, 0xac, 0x5a, 0x57, 0x9e, 0x16, 0xcf, 0x7d, 0xd2, 0xd5, 0x4a, 0x12, 0x86,
        0xd8, 0x79, 0x7a, 0xd9, 0xfd, 0x03, 0x1c, 0xfb, 0x34, 0x86, 0x5a, 0x0d, 0x58, 0xaf, 0x9f, 0x36,
        0x58, 0x22, 0x50, 0x99, 0x14, 0x83, 0xd0, 0x64, 0x74, 0x7a, 0x89, 0xb2, 0x5d, 0xab, 0xe1, 0xb2,
        0xb9, 0xc3, 0x38, 0x87, 0x69, 0x5f, 0xfb, 0x54, 0x58, 0x28, 0x8d, 0x42, 0x67, 0xc7, 0x54, 0x79,
        0x7e, 0xdb, 0xb5, 0x1d, 0x84, 0x16, 0x1a, 0x51, 0x5d, 0x10, 0xe4, 0x41, 0xf1, 0x78, 0x7a, 0x0d,
        0x22, 0xf4, 0x42, 0xde, 0xdd, 0x25, 0xa6, 0xda, 0x05, 0xb4, 0x09, 0x25, 0x7e, 0x47, 0xd4, 0xe7,
        0x30, 0x85, 0x66, 0x49, 0xfd, 0x10, 0x29, 0x6b, 0xd3, 0x92, 0x85, 0x78, 0x8d, 0xb0, 0xdd, 0x91,
        0x1b, 0x0f, 0x9b, 0xe0, 0x5f, 0x58, 0x1f, 0x86, 0x3d, 0x2d, 0xd5, 0xb4, 0xa2, 0x37, 0x1e, 0x84,
        0x6c, 0x18, 0x61, 0x1f, 0x85, 0x7b, 0x09, 0xae, 0xd2, 0x00, 0x75, 0x5a, 0x38, 0x55, 0x0d};
    const std::vector<uint8_t> info{0x42};
    std::unique_ptr<::Botan::KDF> hkdf = nullptr;
    hkdf = ::Botan::KDF::create("HKDF(SHA-256)");
    hkdf->derive_key(1264, password, salt, info);
    return 0;
}

Output:

/usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/debug/array:155:
In function:
    std::array::reference std::array<unsigned char, 
    64>::operator[](std::array::size_type) [_Tp = unsigned char, _Nm = 64]

Error: attempt to subscript container with out-of-bounds index 64, but 
container only holds 64 elements.

Objects involved in the operation:
    sequence "this" @ 0x0x55ec329a3818 {
      type = std::__debug::array<unsigned char, 64ul>;
    }
Aborted (core dumped)

The reason is that m_buffer (which has 64 elements) is accessed at element 64:

clear_mem(&m_buffer[m_position], elements_until_alignment());

It is illegal to access a container with an out-of-bounds index, even if it isn't dereferenced.

@guidovranken
Copy link
Author

Maybe introduced in a997b33 @reneme

@reneme reneme added the bug label Oct 6, 2023
@reneme reneme added this to the Botan 3.2.0 milestone Oct 6, 2023
@reneme reneme self-assigned this Oct 6, 2023
@reneme
Copy link
Collaborator

reneme commented Oct 6, 2023

Thanks! I'll look into it.

@reneme
Copy link
Collaborator

reneme commented Oct 7, 2023

@randombit I'm quite sure we would have spotted this (even without a dedicated reproducer) in CI if we'd use the debug flags that @guidovranken is proposing. For gcc we even have it configured in the build system (as "iterator" sanitizer). But from a quick glance it is not enabled in the "sanitizers" CI job.

I'll look into that next week.

@guidovranken
Copy link
Author

I suggest also enabling it in the Botan OSS-Fuzz project. Use -D_LIBCPP_DEBUG=1 for that.

@thesamesam
Copy link

thesamesam commented Oct 8, 2023

-D_GLIBCXX_DEBUG (if you're using the GNU libc) or to -D_LIBCPP_DEBUG=1 (if you're using the LLVM libc).

Note that it's not about libc, but rather the C++ standard libary used (libstdc++ or libc++). You can use either of these on glibc, musl, etc.

guidovranken added a commit to guidovranken/oss-fuzz that referenced this issue Oct 14, 2023
Enable `-D_LIBCPP_DEBUG=1` randombit/botan#3734
Compile wolfCrypt with AES EAX ciphermode
DavidKorczynski pushed a commit to google/oss-fuzz that referenced this issue Oct 15, 2023
- Enable `-D_LIBCPP_DEBUG=1` to find bugs like
randombit/botan#3734
- Compile wolfCrypt with AES EAX ciphermode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants