[MESOS-7143] ABORT checks its preconditions incorrectly and incompletely

• Type: Bug
• Status: Open
• Priority: Minor
• Resolution: Unresolved
• Affects Version/s: 0.23.0
• Component/s: stout
• Labels: coverity mesosphere tech-debt
• Story Points: 1
• Assignee: Unassigned
• Reporter: Benjamin Bannier
• Created: 18/Feb/17 09:22
• Updated: 20/Jul/17 23:26

Description

Currently, stout's ABORT (which is mapped to _Abort) checks it precondition incompletely and incorrectly.

Its current control flow is roughly

void _Abort(const char* prefix, const char* message)
{
  size_t prefix_len = strlen(prefix);
  size_t message_len = strlen(message);
  
  // Async-safe write.
   while(::write(2, prefix, prefix_len) == -1 && errno == EINTR);
   while(message != nullptr &&
         ::write(2, message, message_len) == -1 && errno == EINTR);
}

We here check the precondition message != nullptr after we already have called strlen(message); calling strlen on a nullptr already triggers undefined behavior.

Similarly, we never guard against a prefix which is nullptr, but unconditionally call strlen on it.

It seems it should be possible to assert that neither prefix nor message are nullptr before any use.

This was diagnosed by coverity as CID-1400833, and has been present in all releases since 0.23.0.