Lawo / ember-plus

Ember+ control protocol - Slick and free for all!
https://github.com/Lawo/ember-plus/wiki
Boost Software License 1.0
112 stars 43 forks source link

libember: Segfault while decoding -0.0 #120

Open stephan-ja opened 1 month ago

stephan-ja commented 1 month ago

libember

Currently the special encoding of -0.0 as described in X.690 (Ch 8.5.9 1 byte, value 0x43) is not handled in libember (and libember-slim) for encoding and decoding and leads to a segmentation fault while decoding as the input length is not checked and the head of the empty stream buffer (which is 0) is dereferenced.

C

In C# the encoding ist defined https://github.com/Lawo/ember-plus-sharp/blob/1a1c2387c90288b8bae311f318b183659e22c1de/Lawo.EmberPlusSharp/Ember/EmberWriter.cs#L363 , so there is a good chance that a C# client will crash a C++ (libember) server.

The current encoding of -0.0 in C++ (libember) will lead to the C# exception https://github.com/Lawo/ember-plus-sharp/blob/1a1c2387c90288b8bae311f318b183659e22c1de/Lawo.EmberPlusSharp/Ember/EmberReader.cs#L563 .

libember_slim

In libember_slim -0.0 will be decoded as NAN https://github.com/Lawo/ember-plus/blob/678838bc0573a7f797c83ba4820d4521e53aaeb9/libember_slim/Source/ber.c#L734 .

needed fixes

KimonHoffmann commented 2 weeks ago

Thank you very much for reporting these issues. The changes merged in PR #121 should comprehensively address both aspects of the issue. Please report back if you discover further issues with proposed solution.

stephan-ja commented 2 weeks ago

The fix added the encoding for -0.0 but didn't change the according length to 1. So the check from https://github.com/Lawo/ember-plus/blob/e6d97fd42afd2bb92c0b2feaed658255c89b82f2/libember/Headers/ember/ber/traits/Real.hpp#L67-L68 has to be added to https://github.com/Lawo/ember-plus/blob/e6d97fd42afd2bb92c0b2feaed658255c89b82f2/libember/Headers/ember/ber/traits/Real.hpp#L105-L109 as well.

The decoding worked and the length check also prevented the segfault. The exception message Not enough data is missing a bit of context to help the lib user to determine where the exception originated from and what caused it.

Also note:

KimonHoffmann commented 1 week ago

Good catch, thank you! A fix has been pushed that should address the remaining issue with encodedLength().

Fixes libemeber_slim and the EmberPlus Viewer are also necessary, but will be treated with slightly lower priority, as they don't lead to crashes in software using the library.