libember: Segfault while decoding -0.0

[stephan-ja]

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.

• ember-plus/libember/Headers/ember/ber/traits/Real.hpp

  Line 179 in 678838b

  long long exponent = ber::decode<long long>(input, exponentLength);

  
  calls
  

  ember-plus/libember/Headers/ember/ber/traits/Integral.hpp

  Line 125 in 678838b

  util::OctetStream::value_type byte = input.front();

  
  with an empty input and segfaults at
  

  ember-plus/libember/Headers/ember/util/StreamBuffer.hpp

  Line 429 in 678838b

  return m_head->at(m_head->first());

  
  while dereferencing m_head (nullptr).

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

ember-plus/libember_slim/Source/ber.c

Line 734 in 678838b

qwValue = 0x7fC00000L;

.

needed fixes

• So en/decoding of -0.0 should be handled (libember / libember_slim), but I'm not sure if -0.0 is really needed or if it shouldn't be silently converted to 0.0 to avoid crashing applications using the current lib. If the silent conversion approach is taken than it probably should also be taken in C#.

• Length should be checked to avoid the segfault, as the standard might be enhanced in the future.

• StreamBuffer::front should probably always check that m_head is not nullptr and throw an exception if it is, to avoid future segmentation faults where the assumptions while using it don't apply.

[KimonHoffmann]

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]

The fix added the encoding for -0.0 but didn't change the according length to 1. So the check from

ember-plus/libember/Headers/ember/ber/traits/Real.hpp

Lines 67 to 68 in e6d97fd

	(value == static_cast<value_type>(0.0)) &&
	util::signbit(value))

has to be added to

ember-plus/libember/Headers/ember/ber/traits/Real.hpp

Lines 105 to 109 in e6d97fd

	if (value == +std::numeric_limits<value_type>::infinity()
	|| value == -std::numeric_limits<value_type>::infinity()
	|| value == std::numeric_limits<value_type>::quiet_NaN()
	|| value == std::numeric_limits<value_type>::signaling_NaN())
	{

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:

• The en/decoding in libember_slim doesn't seem to be addressed (We are not using it but others may).
• EmberPlus Viewer needs to be rebuild with these changes, as it (v2.40.0.35) will currently crash when encountering 0x43 for -0.0.

[KimonHoffmann]

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.