algesten / str0m

A Sans I/O WebRTC implementation in Rust.
MIT License
334 stars 50 forks source link

h264.rs:263:30: index out of bounds: the len is 42 but the index is 42 #586

Open xnorpx opened 2 weeks ago

xnorpx commented 2 weeks ago

I have not checked the code yet, but save it here to not lose it

101072 REDUCE cov: 940 ft: 2951 corp: 279/91Kb lim: 690 exec/s: 12634 rss: 318Mb L: 166/663 MS: 5 InsertRepeatedBytes-EraseBytes-CMP-InsertByte-InsertByte- DE: "\377\377"-

thread '' panicked at /home/runner/work/str0m/str0m/src/packet/h264.rs:263:30: index out of bounds: the len is 42 but the index is 42

https://github.com/algesten/str0m/actions/runs/11769744229/job/32781189675#step:8:916

algesten commented 2 weeks ago
                while curr_offset + 1 < packet.len() {
                    let nalu_size =
                        ((packet[curr_offset] as usize) << 8) | packet[curr_offset + 1] as usize;
                    curr_offset += STAPA_NALU_LENGTH_SIZE;

We check for +1, but STAPA_NALU_LENGTH_SIZE is 2. Maybe that's it?

k0nserv commented 2 weeks ago

We check for +1, but STAPA_NALU_LENGTH_SIZE is 2. Maybe that's it?

At offset 1 we expect two bytes for the length if the packet is 2 bytes long 1 + 1 < 2 == false and we don't loop. The minimum is 3 bytes.

Failing test here https://github.com/algesten/str0m/compare/main...feature/stapa-nalu-parsing. I think this condition is wrong https://github.com/algesten/str0m/blob/36ae0ec699ce6e871470f779ac32609e1d22d533/src/packet/h264.rs#L256-L261, should be >= packet.len(), but that makes another test fail

algesten commented 1 week ago

Is the test wrong?

k0nserv commented 1 week ago

It's possible, I'll dig into it