Comment 2 for bug 2036595

Revision history for this message
kev (kbackhouse2000) wrote :

# GitHub Security Lab (GHSL) Vulnerability Report, libcue: `GHSL-2023-197`

The [GitHub Security Lab](https://securitylab.github.com) team has identified a potential security vulnerability in [libcue](https://github.com/lipnitsk/libcue).

We are committed to working with you to help resolve this issue. In this report you will find everything you need to effectively coordinate a resolution of this issue with the GHSL team.

If at any point you have concerns or questions about this process, please do not hesitate to reach out to us at `<email address hidden>` (please include `GHSL-2023-197` as a reference).

If you are _NOT_ the correct point of contact for this report, please let us know!

## Summary

libcue is a library for parsing [CUE sheet](https://en.wikipedia.org/wiki/Cue_sheet_%28computing%29) files. A malicious file can trigger an out-of-bounds array access in the `track_set_index` function.

## Project

libcue

## Tested Version

[2.2.1](https://github.com/lipnitsk/libcue/releases/tag/v2.2.1)

## Details

### Out of bounds array access in track_set_index (`GHSL-2023-197`)

The function [`track_set_index`](https://github.com/lipnitsk/libcue/blob/1b0f3917b8f908c81bb646ce42f29cf7c86443a1/cd.c#L340-L348) does not check that `i >= 0`:

```c
void track_set_index(Track *track, int i, long ind)
{
 if (i > MAXINDEX) {
  fprintf(stderr, "too many indexes\n");
                return;
        }

 track->index[i] = ind;
}
```

If `i` is negative, then this code can write to an address outside the bounds of the array.

The value of `i` is parsed using [`atoi`](https://en.cppreference.com/w/c/string/byte/atoi) in [`cue_scanner.l`](https://github.com/lipnitsk/libcue/blob/1b0f3917b8f908c81bb646ce42f29cf7c86443a1/cue_scanner.l#L132):

```c
[[:digit:]]+ { yylval.ival = atoi(yytext); return NUMBER; }
```

`atoi` does not check for integer overflow, so it is easy to get it produce a negative number.

This is an example CUE file which triggers the bug:

```
FILE pwned.mp3 MP3
TRACK 000 AUDIO
INDEX 4294567296 0
```

The index `4294567296` is converted to `-400000` by `atoi`.

#### Impact

This issue may lead to code execution when libcue is used to parse a malicious file.

#### Remediation

Suggested fix:

```
diff --git a/cd.c b/cd.c
index cf77a18..4bbea19 100644
--- a/cd.c
+++ b/cd.c
@@ -339,7 +339,7 @@ track_get_rem(const Track* track)

 void track_set_index(Track *track, int i, long ind)
 {
- if (i > MAXINDEX) {
+ if (i < 0 || i > MAXINDEX) {
                fprintf(stderr, "too many indexes\n");
                 return;
         }
```

## GitHub Security Advisories

We recommend you create a private [GitHub Security Advisory](https://help.github.com/en/github/managing-security-vulnerabilities/creating-a-security-advisory) for this finding. This also allows you to invite the GHSL team to collaborate and further discuss this finding in private before it is [published](https://help.github.com/en/github/managing-security-vulnerabilities/publishing-a-security-advisory).

## Credit

This issue was discovered and reported by GHSL team member [@kevinbackhouse (Kevin Backhouse)](https://github.com/kevinbackhouse).

## Contact

You can contact the GHSL team at `<email address hidden>`, please include a reference to `GHSL-2023-197` in any communication regarding this issue.

## Disclosure Policy

This report is subject to a 90-day disclosure deadline, as described in more detail in our [coordinated disclosure policy](https://securitylab.github.com/advisories#policy).