fix(events): fix dns events slice out of bounds issues

[rafaeldtinoco]

Fixes: #3547

[yanivagman]

It seems to me that the issue here is an "off by one" error, and I don't understand how this PR fixes it.
Can you please explain what you think is the root cause here? Were you able to reproduce it?

Since it doesn't seem that the derive function of the dns event accesses a slice directly, my guess would have been that some null byte was added/removed somewhere when converting from bytes to string or vice versa, however, your PR doesn't touch such places, so I'm not convinced it solves the issue

[rafaeldtinoco]

Can you please explain what you think is the root cause here? Were you able to reproduce it?

There is so many things that could be happening =). When DNS messages are too large to fit the regular UDP packet, the DNS message might be truncated. There is a flag in the DNS header (TC) for truncation but its better to try to parse the packet and get the info you have. This can lead to the situation where the slice would be smaller than what was advertised by the packet header.

By ranging instead of using the index directly we would get rid of the out of bounds errors and still be able to range over what came in a truncated response, for example.

This is just one of the things I can think could be happening. Still, all the things would lead to out of bounds and by not using the index I'm probably getting rid of the error and allowing the event to continue (for other healthy DNS events, for example).

I was not able to reproduce this error and I would love whoever had this issue to test the patch and let me know the outcome.

[yanivagman]

Can you please explain what you think is the root cause here? Were you able to reproduce it?

There is so many things that could be happening =). When DNS messages are too large to fit the regular UDP packet, the DNS message might be truncated. There is a flag in the DNS header (TC) for truncation but its better to try to parse the packet and get the info you have. This can lead to the situation where the slice would be smaller than what was advertised by the packet header.

By ranging instead of using the index directly we would get rid of the out of bounds errors and still be able to range over what came in a truncated response, for example.

This is just one of the things I can think could be happening. Still, all the things would lead to out of bounds and by not using the index I'm probably getting rid of the error and allowing the event to continue (for other healthy DNS events, for example).

I was not able to reproduce this error and I would love whoever had this issue to test the patch and let me know the outcome.

Note that the issue indicated an out of bounds error when trying to access a slice and not a specific index. That's why I'm not sure that using range will solve the issue

[rafaeldtinoco]

runtime error: slice bounds out of range [:52] with capacity 51"}

I believe this type of address, and the explanation I've given, can mitigate the issue because with appends there won't be specific indexes to slices (and where to put data on them).

I'm asking @geyslan for a review.. and, even if the fix doesn't work, we can try something else later. The change won't cause any harm.

[geyslan]

LGTM, it seems to preserve behaviour.

Just one question, since it's related to out of bounds, I ponder: Is three a chance that the passed slices (underlying arrays) are changing between len() and range?

[rafaeldtinoco]

Is three a chance that the passed slices (underlying arrays) are changing between len() and range?

I don't think so, they're using for a single packet (event) parsing, no concurrent access and allocation and usage are very close. But, like said before, I'm not sure it will address the issue (we don't have a reproducer) but it might if the theory is correct. If not, then we find something else!

THanks!