Talos Vulnerability Report

TALOS-2023-1744

Diagon Sequence::DrawText heap-based buffer overflow vulnerability

July 5, 2023
CVE Number

CVE-2023-27390

SUMMARY

A heap-based buffer overflow vulnerability exists in the Sequence::DrawText functionality of Diagon v1.0.139. A specially crafted markdown file can lead to arbitrary code execution. A victim would need to open a malicious file to trigger this vulnerability.

CONFIRMED VULNERABLE VERSIONS

The versions below were either tested or verified to be vulnerable by Talos or confirmed to be vulnerable by the vendor.

Diagon v1.0.139

PRODUCT URLS

Diagon - https://github.com/ArthurSonzogni/Diagon

CVSSv3 SCORE

7.8 - CVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H

CWE

CWE-122 - Heap-based Buffer Overflow

DETAILS

Diagon is an interpreter that translates markdown in several formats: latex, planar graph, table, tree and many others.

The Diagon interpreter translates a markdown text sequence diagram to a graphical sequence diagram. For instance, the input could look like this:

Left -> Right: Message1
Left <- Right: Message2

and the sequence diagram would result in:

 ┌────┐  ┌─────┐
 │Left│  │Right│
 └─┬──┘  └──┬──┘
   │        │
   │Message1│
   │───────>│
   │        │
   │Message2│
   │<───────│
 ┌─┴──┐  ┌──┴──┐
 │Left│  │Right│
 └────┘  └─────┘

The input will be parsed and will eventually reach the Sequence::UniformizeActors function:

void Sequence::UniformizeActors() {
  [...]

  for (auto& message : messages) {
    for (auto& actor : {message.from, message.to}) {
      if (!actor_index.count(actor)) {
        actor_index[actor] = actors.size();
        Actor a;
        a.name = actor;
        actors.push_back(a);
      }
    }
  }
}

This function will parse the actors in each message in messages, which consist of each row of the sequence diagram text. The actors are the sender and the receiver. In the example above, the actors are “Left” and “Right.” The function populates the actors vector and associates, in the actor_index map, the name of the actor with the number of elements in the actors vector at the creation time of the entry. This essentially can be considered an index starting from 0 and increasing with each new actor.

Eventually the Sequence::LayoutComputeActorsPositions function will be called to calculate the position of the various actors in the sequence diagram:

void Sequence::LayoutComputeActorsPositions() {
  [...]
  
  for (auto& message : messages) {
    ActorSpace space{actor_index[message.from],  //
                     actor_index[message.to],    //
                     message.width + 1};         //
    if (space.a > space.b)
      std::swap(space.a, space.b);
    spaces.push_back(space);
  }

  if (actors.size() != 0)
    actors[0].center = actors[0].name.size() / 2 + 1;

  bool modified = true;
  int i = 0;
  while (modified) {
    modified = false;
    for (const ActorSpace& s : spaces) {
      if (actors[s.b].center - actors[s.a].center < s.space) {                                                  [1]
        actors[s.b].center = actors[s.a].center + s.space;
        modified = true;
      }
    }
    if (i++ > 500) {                                                                                            [2]
      std::cout << "Something went wrong!" << std::endl;
      break;
    }
  }

  for (auto& actor : actors) {                                                                                  [3]
    actor.left = actor.center - actor.name.size() / 2 - actor.name.size() % 2;
    actor.right = actor.left + actor.name.size() + 2;
  }
}

Then, using the calculated positions, a Screen class is instantiated:

std::string Sequence::Draw() {
    // Estimate output dimension.
    int width = actors.back().right;
    int height = 0;
    for (const Message& message : messages) {
      height = std::max(height, message.bottom);
    }
    height += 4;

    Screen screen(width, height);

    for (auto& actor : actors)
      actor.Draw(screen, height);

    for (auto message : messages)
      message.Draw(screen);

    if (ascii_only_)
      screen.ASCIIfy(0);
    return screen.ToString();
}

The Screen class contains the lines_ matrix where the graphical sequence diagram is going to be written. This matrix takes into consideration the various sizes involved: the actors, the arrows, the messages, etc. Eventually the names of the actors and the messages between the them will be “drawn.” For each of these texts the Screen::DrawText function will be called:

void Screen::DrawText(int x, int y, std::wstring_view text) {
    for (auto& c : text)
        lines_[y][x++] = c;                                                                                     [4]
} Here `lines_` is the member of the `Screen` class instantiated in `Sequence::Draw`.

Sequence::LayoutComputeActorsPositions uses a variable called spaces. This is a vector dictating the minimum spaces between two actors. The spaces vector contains various instances of the ActorSpace struct. This struct contains two indexes for specifying which actors are involved: the ones stored in the actor_index map, and an integer value that represents the minimum spaces required between the two actors. For each message, one struct is created, using the minimum space required to write the message between the two actors. The spaces vector also contains the spaces required for each pair of adjacent actors. Each actor has three parameters to satisfy this space requirements: left, center and right. These dictate the position of each actor in the diagram.

To simplify the code explanation following, we are going to omit that the spaces array has the distances also for the adjacent actors and consider only the spaces in relationship to the messages and their sender and receiver actors. The Sequence::LayoutComputeActorsPositions function executes a while loop to modifying the actors positions based on the content of spaces. The while is executed until either the condition actors[s.b].center - actors[s.a].center < s.space, at [1], is false, meaning that the space between the two actors is enough for the message to be drawn with no modification of the actors positions, or the while has been executed 500 times, a check is executed at [2], and something was wrong, as suggested by the print.

This function has a bug, which can lead to a heap buffer overflow. Let’s take for example the following representation of a sequence diagram:

  A -> A:Message

In this example the sender and the receiver are the same actor, meaning that the index of the two will be equal; in this example, 0. The condition at [1], actors[s.b].center - actors[s.a].center < s.space for this example, can be simplified as actors[0].center - actors[0].center < s.space. Because the portion on the left is always equal to 0, this condition is always true. This means that the actors[s.b].center value is always updated without progress for making the condition at [1] false. Furthermore, also at [3], the actor’s left and right positions are updated, increasing their value.

Eventually the code at [4] is reached. The variable lines_ has a width and a height that are based on the positions calculated for the actors and the messages. The problem is that, even if the while in Sequence::LayoutComputeActorsPositions has been executed 500 times, no progress on changing the position of the actor to accommodate the message has been made. Indeed, the actor’s center, left and right values increased substantially, but the distance between the values did not. In this example the only actor present has the following position values:

  (gdb) p actors
  $19 = std::vector of length 1, capacity 1 = {{
      [...]
      left = 0xfb0,
      center = 0xfb1,
      right = 0xfb3
    }}

The left is 1 value from center, which is 2 value away from right. This is due to the calculation made at [3]. The space between left and right should take into consideration the length of the message, but because of the bug just explained it was not possible to account for that. This can lead to a heap buffer overflow vulnerability at [4] writing the message out-of-bounds.

Exploit Proof of Concept

Following the content of the POC used:

$ cat POC.md
A->A:POC_BOF

If we run Diagon compilation with ASAN with the POC:

$ diagon Sequence < POC.md

Something went wrong!
=================================================================
==256826==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x628000017fd0 at pc 0x000000c77eab bp 0x7fff5f481050 sp 0x7fff5f481048
WRITE of size 4 at 0x628000017fd0 thread T0
    #0 0xc77eaa in Screen::DrawText(int, int, std::basic_string_view<wchar_t, std::char_traits<wchar_t> >) /home/vagrant/Diagon/src/screen/Screen.cpp:32:20
    #1 0x8a1515 in Message::Draw(Screen&) /home/vagrant/Diagon/src/translator/sequence/Sequence.cpp:71:12
    #2 0x8aefb0 in Sequence::Draw[abi:cxx11]() /home/vagrant/Diagon/src/translator/sequence/Sequence.cpp:692:13
    #3 0x8acc37 in Sequence::Translate(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/vagrant/Diagon/src/translator/sequence/Sequence.cpp:293:10
    #4 0x4fbd0e in (anonymous namespace)::Translate(Translator*, int, char const**) /home/vagrant/Diagon/src/main.cpp:277:36
    #5 0x4f97e7 in main /home/vagrant/Diagon/src/main.cpp:326:12
    #6 0x7f1a7e31ad09 in __libc_start_main csu/../csu/libc-start.c:308:16
    #7 0x44ca69 in _start (/home/vagrant/Diagon/build/diagon-1.0.139+0x44ca69)

0x628000017fd0 is located 0 bytes to the right of 16080-byte region [0x628000014100,0x628000017fd0)
allocated by thread T0 here:
    #0 0x4f677d in operator new(unsigned long) (/home/vagrant/Diagon/build/diagon-1.0.139+0x4f677d)
    #1 0x7f1a7e655a0e in void std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::_M_construct<wchar_t*>(wchar_t*, wchar_t*, std::forward_iterator_tag) (/lib/x86_64-linux-gnu/libstdc++.so.6+0x14ba0e)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/vagrant/Diagon/src/screen/Screen.cpp:32:20 in Screen::DrawText(int, int, std::basic_string_view<wchar_t, std::char_traits<wchar_t> >)
Shadow bytes around the buggy address:
  0x0c507fffafa0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fffafb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fffafc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fffafd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fffafe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c507fffaff0: 00 00 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa
  0x0c507fffb000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c507fffb010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c507fffb020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fffb030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fffb040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==256826==ABORTING

We can see that a heap buffer overflow occured at Sequence.cpp:71 that corresponds to the code at [4].

VENDOR RESPONSE

The maintainer has provided a patch at: https://github.com/ArthurSonzogni/Diagon/releases/tag/v1.1.158

TIMELINE

2023-05-03 - Vendor Disclosure
2023-05-08 - Vendor Patch Release
2023-07-05 - Public Release

Credit

Discovered by Francesco Benvenuto of Cisco Talos.