error when decoding compressed domain name

[jiangzhuti]

RFC 1035 4.1.4 says:

The compression scheme allows a domain name in a message to be represented as either:

• a sequence of labels ending in a zero octet
• a pointer
• a sequence of labels ending with a pointer

so when decoding a dns packet which contains a pointer-ending CNAME at the end, the code runs here: https://github.com/mnezerka/dnslib/blob/master/src/buffer.cpp#L287
saveBuffPos is equal to the buffer's size, then, at line 290 setPos(saveBuffPos);, an exception is thrown.

[mnezerka]

Hi @jiangzhuti, thank you for all your findings. If you already have a fix, don't hesitate to come with PR. I'll appreciate it since I'm no longer working in area of telecommunication test automation and it will definitely take some time to refresh my knowledge of those RFCs and fix it properly.

[jiangzhuti]

Hi @jiangzhuti, thank you for all your findings. If you already have a fix, don't hesitate to come with PR. I'll appreciate it since I'm no longer working in area of telecommunication test automation and it will definitely take some time to refresh my knowledge of those RFCs and fix it properly.

ok, I will try. This is an interesting project.

[mnezerka]

To be able to validate your PRs and decision to change setPos(), would it be possible to share an example of call which is causing problems? I mean - fragment of code (or unit test) that fails for your "pointer-ending CNAME" packet? I could spend some time to thing about this case and see the execution flow. Allowing of buffer to point at byte which doesn't belong to it is something I'm trying to avoid. May be, there is a different approach to solve this issue - e.g. check if buffer is full or something similar.

[wxiaoguang]

Thank you for the great library.

I think I have fixed the problem here: https://github.com/wxiaoguang/dnslib-cpp/blob/42a598c2d19e4357e9d90c079aa2d2d70654b5de/dnslib/buffer.cpp#L342

I did some rewriting to the old code, to make the "seek/movePtr (old: setPos)" support "set the position to the end". That's the the widely-used behavior for a buffer reader.

[mnezerka]

Thank you for the great library.

I think I have fixed the problem here: https://github.com/wxiaoguang/dnslib-cpp/blob/42a598c2d19e4357e9d90c079aa2d2d70654b5de/dnslib/buffer.cpp#L342

I did some rewriting to the old code, to make the "seek/movePtr (old: setPos)" support "set the position to the end". That's the the widely-used behavior for a buffer reader.

Hi,

I'll appreciate if you create PR with the fix.

Thank you.

[wxiaoguang]

The problem is more complicated that it looks, and I am not able to "fix" it with this code base. So I rewrote a lot of code and made a hard fork, also and fixed other bugs including invalid name compression result. The newly written code changed too many function definitions.

[jiangzhuti]

The problem is more complicated that it looks, and I am not able to "fix" it with this code base. So I rewrote a lot of code and made a hard fork, also and fixed other bugs including invalid name compression result. The newly written code changed too many function definitions.

感谢兄弟