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

chacha20 NEON Bug: 64-bit counter issue #381

Open
nstilt1 opened this issue Jan 1, 2025 · 1 comment
Open

chacha20 NEON Bug: 64-bit counter issue #381

nstilt1 opened this issue Jan 1, 2025 · 1 comment

Comments

@nstilt1
Copy link
Contributor

nstilt1 commented Jan 1, 2025

While I was developing the 64-bit counter variant, I had a suspicion that the NEON code used 64-bit addition with the counter and I wrote a test for it. The test I wrote passed with all backends, except for the NEON backend, resulting in a mismatch between the output of the first four blocks and the output of the first four blocks after the counter was supposed to wrap around.

This would not have affected the cipher implementations because they panic when it reaches the end of the keystream, but it does affect the rng.

This is not a vulnerability; however, it is still a bug, and it should probably be dealt with. It can probably be fixed by changing add64! to add32!, but when I was developing the 64-bit counter, I had to change the order of the operations when the counter was being applied to maintain the functionality of both counters. But it can probably be fixed by changing that macro if we only need the 32-bit counter.

This is the test that failed on NEON and passed on the other backends:

    #[test]
    fn counter_wrapping() {
        let mut rng = ChaChaRng::from_seed([0u8; 32]);

        // get first four blocks and word pos
        let mut first_blocks = [0u8; 64 * 4];
        rng.fill_bytes(&mut first_blocks);
        let word_pos = rng.get_word_pos();

        // get first four blocks after wrapping
        rng.set_block_pos(u32::MAX);
        let mut result = [0u8; 64 * 5];
        rng.fill_bytes(&mut result);
        assert_eq!(word_pos, rng.get_word_pos());
        assert_eq!(&first_blocks[0..64 * 4], &result[64..]);
    }
@nstilt1
Copy link
Contributor Author

nstilt1 commented Jan 1, 2025

I don't know if I created this bug when I rewrote the NEON backend, but all I do know is that it's there now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant