# VLC 3.x out of bounds read in seconds

Note
References of secstotimestr were from before it was renamed to vlc_tick_to_str as a result of a commit found here.
i_seconds was also renamed to ticks with the type vlc_tick_t in this commit. For the purposes of keeping it simple, the parameter change will not be reflected here.

## Expectations of specially crafted WEBM

Like any other day on Discord, I would come across specially crafted WEBM files that would mess around with the duration of the video.
This usually results in media players:

• Video duration being the same as the current duration count (incrementing as the play time exceeds the fake video duration time)
• Videos being able to seemingly stop early
• Very long video duration that is never reached (video stops at its actual total playback time)

## The unusual encounter

On January 14 2021, I had came across a video named chinax.webm (can be downloaded here).
When playing back the video on Discord in the Electron desktop application, it presented something I had never seen before. The times shown were in the negatives.

On Google Chrome, this shown slightly different behaviour.

It is worth noting that Discord uses a modified version of the media player. Yes, this did result in some unique bugs that went as far as crashing the entire client.

Now, what about VLC media player? Lets check that one ou–

Ah, right.

## Digging in

My first attempt to try debugging this was by running VLC via CMD with the verbose flag. There was no output at all. It so turns out that if I want a clearer picture of what is going on, I have to use something else.

I had spun up a Ubuntu virtual machine. I then re-attempted. It said nothing really new. Just that there was a segmentation fault.

Evidently, not very useful. Plan B? GDB. That is the correct approach to debugging such software.

This revealed that the at-the-time secstotimestr was causing the application to crash.

GDB also helped reveal the fake duration that would have been shown at first.

## The source

Reviewing the commit found here (src/misc/mtime.c), the code looked very off.

To get a better understanding of the issue, I decided to compile a few builds.

## VLC 4 dev compilation and testing

My initial assumption was that the long unreleased VLC 4 dev build would be affected as it appeared to have the same code. Au contraire, mon ami.

VLC 4 appeared to be unaffected. This may have been as a result of the adjustments made under this commit.

## VLC 3 compilation and debugging

After compiling (unoptimised) the most recent VLC 3 build at the time, I tested the video again and it still crashed. I then went and used GDB. This painted a clear image as to what was going on.

## The bug

There was an integer overflow with i_seconds of which was using the type int32_t (32-bit signed integer).
The urinary operator has undefined behaviour when it comes to overflowed or underflowed integers. Basically, it ended up not making the integer into a positive number and so it remained in the negatives.

As i_seconds would remain in the negatives, it would be in a position where it would keep meeting the condition and the function would recursively call itself. It would as a result keep incrementing and going through the buffer. This goes through the application’s own memory. Eventually, it causes a memory violation and then the application’s process ends with SIGSEGV signal (segmentation fault).

## Further visualising the issue

To better observe the behaviour, I had created a test program with the specific affected parts of the code.

This had resulted in the following output:

## A second pair of eyes

At the time of writing, there is this trend of using the GPT-3.5 based ChatGPT AI (while it is still ‘free’).

There had been talk about it writing code as well as identifying issues. So, why not give it my test code as something to review?

The following is a snippet of what came back after the prompt to look out for any security vulnerabilities or bugs:

This is not completely accurate but it mostly is. More than enough to raise red flags that this was not written safely.

## Severity of the issue

Considering that there are protections against ROP (return-oriented programming), I do not see a clear path of exploiting this issue through a specially crafted video file when it comes to production builds.
If anything, this is a lesson of what not to do in C++.. or any memory unsafe languages for that matter.