memory corruption bugs in libsane
# GitHub Security Lab (GHSL) Vulnerability Report: `GHSL-2020-075`, `GHSL-2020-079`, `GHSL-2020-080`, `GHSL-2020-081`, `GHSL-2020-082`, `GHSL-2020-083`, `GHSL-2020-084`
The [GitHub Security Lab](https://securitylab.github.com) team has identified potential security vulnerabilities in [libsane](http://www.sane-project.org/).
We are committed to working with you to help resolve these issues. In this report you will find everything you need to effectively coordinate a resolution of these issues 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 `securitylab@github.com` (please include the relevant GHSL IDs as a reference).
If you are _NOT_ the correct point of contact for this report, please let us know!
## Summary
libsane contains several memory corruption vulnerabilities which can be triggered by a malicious device or computer that is connected to the same network. The vulnerabilities are triggered when an application such as [simple-scan](http://manpages.ubuntu.com/manpages/bionic/man1/simple-scan.1.html) searches the network for scanners. In the specific case of simple-scan, this happens immediately when simple-scan starts, so there isn't even any need to trick the user into thinking that the scanner is genuine so that they will click on it.
We have also identified some other vulnerabilities in libsane, which are less severe because they _do_ require the user to click the "Scan" button after connecting to a malicious device. We have included these additional bugs in this report. Please note that we have not explored this "one click" attack surface as thoroughly as we have explored the "zero clicks" attack surface, so there may be other bugs in the code which we have not discovered. We also haven't explored the attack surface that would require the malicious device to be connected via USB, rather than the network.
## Product
libsane
## Tested Version
libsane 1.0.27-1~experimental3ubuntu2.2, tested on Ubuntu 18.04.4 LTS with simple-scan 3.28.0-0ubuntu1.
## Details
### Issue 1 (`GHSL-2020-075`): null pointer dereference in `sanei_epson_net_read`
The function [`sanei_epson_net_read`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epson2_net.c#L66) has buggy code for handling the situation where it receives a response with an unexpected size:
```c
/* receive net header */
size = sanei_epson_net_read_raw(s, header, 12, status);
if (size != 12) {
return 0;
}
if (header[0] != 'I' || header[1] != 'S') {
DBG(1, "header mismatch: %02X %02x\n", header[0], header[1]);
*status = SANE_STATUS_IO_ERROR;
return 0;
}
size = be32atoh(&header[6]); <===== size is controlled by attacker
DBG(23, "%s: wanted = %lu, available = %lu\n", __func__,
(u_long) wanted, (u_long) size);
*status = SANE_STATUS_GOOD;
if (size == wanted) {
DBG(15, "%s: full read\n", __func__);
read = sanei_epson_net_read_raw(s, buf, size, status);
if (s->netbuf) {
free(s->netbuf);
s->netbuf = NULL;
s->netlen = 0;
}
if (read < 0) {
return 0;
}
/* } else if (wanted < size && s->netlen == size) { */
} else {
DBG(23, "%s: partial read\n", __func__);
read = sanei_epson_net_read_raw(s, s->netbuf, size, status); <===== s->netbuf could be NULL
if (read != size) {
return 0;
}
s->netlen = size - wanted;
s->netptr += wanted;
read = wanted;
DBG(23, "0,4 %02x %02x\n", s->netbuf[0], s->netbuf[4]); <===== s->netbuf could be NULL
DBG(23, "storing %lu to buffer at %p, next read at %p, %lu bytes left\n",
(u_long) size, s->netbuf, s->netptr, (u_long) s->netlen);
memcpy(buf, s->netbuf, wanted);
}
return read;
```
Notice that the value of `size` is read from an incoming message, so an attacker can set it to any value they like. In the `else` branch, which handles the case where `size != wanted`, there is no check that `s->netbuf` is large enough for `size` bytes. This could potentially lead to a buffer overflow. However, our proof-of-concept exploit for this bug triggers a case where `s->netbuf` hasn't even been initialized so the program crashes due to a null pointer dereference, rather than a buffer overflow.
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
./fakescanner epson 0
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
simple-scan
```
You should see `simple-scan` crash with a segmentation fault.
#### Impact
This issue may lead to remote denial of service, where "remote" means a device or computer connected to the same network as the victim. For example, in a typical office environment the malicious device would need to be somewhere inside the building. Because the vulnerability causes `simple-scan` to crash as soon as it starts, it makes the application unusable.
#### Remediation
The `else` branch of `sanei_epson_net_read`, which handles the case where `size != wanted`, should check that `s->netbuf` has been initialized and that it is large enough for `size` bytes.
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
### Issue 2 (`GHSL-2020-079`): null pointer dereference in `epsonds_net_read`
The function [`epsonds_net_read`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-net.c#L66) has buggy code for handling the situation where it receives a response with an unexpected size:
```c
/* receive net header */
size = epsonds_net_read_raw(s, header, 12, status);
if (size != 12) {
return 0;
}
if (header[0] != 'I' || header[1] != 'S') {
DBG(1, "header mismatch: %02X %02x\n", header[0], header[1]);
*status = SANE_STATUS_IO_ERROR;
return 0;
}
// incoming payload size
size = be32atoh(&header[6]);
DBG(23, "%s: wanted = %lu, available = %lu\n", __func__,
(u_long) wanted, (u_long) size);
*status = SANE_STATUS_GOOD;
if (size == wanted) {
DBG(15, "%s: full read\n", __func__);
if (size) {
read = epsonds_net_read_raw(s, buf, size, status);
}
if (s->netbuf) {
free(s->netbuf);
s->netbuf = NULL;
s->netlen = 0;
}
if (read < 0) {
return 0;
}
} else if (wanted < size) {
DBG(23, "%s: long tail\n", __func__);
read = epsonds_net_read_raw(s, s->netbuf, size, status); <===== no bounds check
if (read != size) {
return 0;
}
memcpy(buf, s->netbuf, wanted);
read = wanted;
free(s->netbuf);
s->netbuf = NULL;
s->netlen = 0;
} else {
DBG(23, "%s: partial read\n", __func__);
read = epsonds_net_read_raw(s, s->netbuf, size, status);
if (read != size) {
return 0;
}
s->netlen = size - wanted; <===== negative integer overflow (because size < wanted)
s->netptr += wanted;
read = wanted;
DBG(23, "0,4 %02x %02x\n", s->netbuf[0], s->netbuf[4]);
DBG(23, "storing %lu to buffer at %p, next read at %p, %lu bytes left\n",
(u_long) size, s->netbuf, s->netptr, (u_long) s->netlen);
memcpy(buf, s->netbuf, wanted); <===== no bounds check
}
return read;
```
This code is very similar to the code in `epson2_net.c` (see issue 1) and has similar bugs. The first of these is a NULL pointer exception at [epsonds-net.c, line 160](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-net.c#L160).
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
./fakescanner epson 1
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
simple-scan
```
You should see `simple-scan` crash with a segmentation fault.
#### Impact
This issue may lead to remote denial of service, where "remote" means a device or computer connected to the same network as the victim. For example, in a typical office environment the malicious device would need to be somewhere inside the building. Because the vulnerability causes `simple-scan` to crash as soon as it starts, it makes the application unusable.
#### Remediation
The `else` branch of `epsonds_net_read`, which handles the case where `wanted > size`, should check that `s->netbuf` has been initialized and that it is large enough for `wanted` bytes. There is also a negative integer overflow in the calculation of `s->netlen`, which should be fixed.
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
### Issue 3 (`GHSL-2020-080`): heap buffer overflow in `epsonds_net_read`
This bug is in the same function as issue 2: `epsonds_net_read`. There is a heap buffer overflow at [epsonds-net.c, line 135](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-net.c#L135). The value of `size` is controlled by the attacker, so an arbitrary amount of attacker-controlled data is written to `s->netbuf`.
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
./fakescanner epson 2
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
simple-scan
```
You should see `simple-scan` crash with a segmentation fault.
#### Impact
This issue may lead to remote code execution, where "remote" means a device or computer connected to the same network as the victim. For example, in a typical office environment the malicious device would need to be somewhere inside the building.
Mitigations such as ASLR may make it difficult for an attacker to create a reliable RCE exploit for the vulnerability. The PoC that we have provided only causes libsane to crash.
#### Remediation
The `else` branch of `epsonds_net_read`, which handles the case where `wanted < size`, should check that `s->netbuf` has been initialized and that it is large enough for `size` bytes.
#### Resources
We have attached the source code for our PoC, fakescanner.cpp, to this report.
### Issue 4 (`GHSL-2020-081`): reading uninitialized data in `epsonds_net_read`
This bug is in the same function as issue 2: `epsonds_net_read`. The `memcpy` at [epsonds-net.c, line 164](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-net.c#L164) can read uninitialized data. The value of `size` is controlled by the attacker, so the attacker can specify that `size == 0`. Since `s->netbuf` is a newly allocated heap buffer, it contains uninitialized memory.
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
./fakescanner epson 3
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
$ gdb simple-scan
(gdb) break epsonds-net.c:164
(gdb) run
```
You need to run `simple-scan` in a debugger (gdb) to see this bug, because it does not cause a crash.
#### Impact
By itself, the severity of this issue is very low. However, it may be very useful to an attacker who is attempting to exploit one of the buffer overflow vulnerabilities, such as issue 3, because it may enable the attacker to obtain the ASLR offsets of the program.
#### Remediation
The `else` branch of `epsonds_net_read`, which handles the case where `wanted > size`, should copy at most `size` bytes into `buf`, because the remaining bytes are uninitialized.
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
### Issue 5 (`GHSL-2020-082`): out-of-bounds read in `decode_binary`
The function [`decode_binary`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-cmd.c#L258) has an out-of-bounds read:
```c
/* h000 */
static char *decode_binary(char *buf)
{
char tmp[6];
int hl;
memcpy(tmp, buf, 4);
tmp[4] = '\0';
if (buf[0] != 'h')
return NULL;
hl = strtol(tmp + 1, NULL, 16);
if (hl) {
char *v = malloc(hl + 1);
memcpy(v, buf + 4, hl);
v[hl] = '\0';
return v;
}
return NULL;
}
```
The value of `hl` is controlled by the attacker and can be any value between 0 and 0xFFF (4095). `buf` is a pointer to a 64 byte stack buffer (`rbuf` in [`esci2_cmd`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-cmd.c#L136)), so the memcpy at line 273 can copy up to 4095 bytes from the stack into the newly allocated buffer.
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
./fakescanner epson 4
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
$ gdb simple-scan
(gdb) break decode_binary
(gdb) run
```
You need to run `simple-scan` in a debugger (gdb) to see this bug, because it does not cause a crash.
#### Impact
By itself, the severity of this issue is very low. However, it may be very useful to an attacker who is attempting to exploit one of the buffer overflow vulnerabilities, such as issue 3, because it may enable the attacker to obtain the ASLR offsets of the program.
#### Remediation
`decode_binary` should check that `hl` does not exceed the remaining number of bytes in the buffer.
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
### Issue 6: SIGPIPE in `sanei_tcp_write`
This issue is not a security issue. It is just a minor coding issue which we noticed and which we thought we would mention since it is very easy to fix. [`sanei_tcp_write`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/sanei/sanei_tcp.c#L118) calls `send`, which can raise a `SIGPIPE` signal if the socket connection has closed. It is not a serious issue because libsane has a signal handler for `SIGPIPE`. We only noticed it because we were running libsane in gdb and saw the signal.
We have included a proof-of-concept for this issue. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
./fakescanner epson 5
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
$ gdb simple-scan
(gdb) run
```
You should see gdb pause the program because a `SIGPIPE` was raised.
#### Impact
This issue does not have any security impact.
#### Remediation
We recommend passing `MSG_NOSIGNAL` as the `flags` argument of `send`, so that it will return an error code rather than raising a `SIGPIPE`. In other words, change the implementation of `sanei_tcp_write` as follows:
```c
ssize_t
sanei_tcp_write(int fd, const u_char * buf, int count)
{
return send(fd, buf, count, MSG_NOSIGNAL);
}
```
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
### Issue 7 (`GHSL-2020-083`): out-of-bounds read in `esci2_check_header`
[`esci2_check_header`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-cmd.c#L93) uses `sscanf` to read a number from the message:
```c
/* INFOx0000100#.... */
/* read the answer len */
if (buf[4] != 'x') {
DBG(1, "unknown type in header: %c\n", buf[4]);
return 0;
}
err = sscanf(&buf[5], "%x#", more);
if (err != 1) {
DBG(1, "cannot decode length from header\n");
return 0;
}
```
`buf` is a pointer to a 64 byte stack buffer (`rbuf` in [`esci2_cmd`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-cmd.c#L136)), the contents of which are entirely controlled by the attacker. If the attacker fills the buffer with the character '0', then `sscanf` will read off the end of the buffer. If the characters beyond the buffer are valid hexadecimal digits, then they will be converted to a number and written to the variable named `more`. The value of `more` is included in the next message that is sent to the malicious device, so this issue is an information leak vulnerability.
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
./fakescanner epson 6
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
$ gdb simple-scan
(gdb) break epsonds-cmd.c:120
(gdb) run
```
You need to run `simple-scan` in a debugger (gdb) to see this bug, because it does not cause a crash.
#### Impact
This issue may lead to remote information disclosure, where "remote" means a device or computer connected to the same network as the victim. In practice, though, this issue is very low severity, because the byte immediately following the buffer is usually not a valid hexadecimal digit, so no information disclosure occurs.
#### Remediation
Make sure that the string in the buffer is null-terminated, so that `sscanf` cannot read beyond the end of the buffer.
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
### Issue 8: integer overflow in the count parameter of `sanei_tcp_read`
The type of the `count` argument of [`sanei_tcp_read`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/sanei/sanei_tcp.c#L124) is `int`, but it is sometimes called with an argument of type `ssize_t`, which can cause an integer overflow.
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
./fakescanner epson 7
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
$ gdb simple-scan
(gdb) break epsonds_net_read_raw
(gdb) cond 1 (wanted > 10000)
(gdb) run
```
You need to run `simple-scan` in a debugger (gdb) to see this bug, because it does not cause a crash. After gdb has stopped at the conditional breakpoint, you should be able to see that the value of `wanted` is 4294967295. But when you step into `sanei_tcp_read` the value of `count` is -1 due to the integer overflow.
We think it is also worth mentioning that the error condition is ignored in `esci2_cmd` ([epsonds-cmd.c, line 195](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-cmd.c#L195)):
```c
ssize_t read = eds_recv(s, pbuf, more, &status);
if (read != more) {
}
/* parse the received data block */
if (cb) {
status = esci2_parse_block(pbuf, more, userdata, cb);
if (status != SANE_STATUS_GOOD) {
DBG(1, "%s: %4s error while parsing received data block\n", __func__, cmd);
}
}
```
So the call to `esci2_parse_block` on line 200 is reading uninitialized data.
#### Impact
The integer overflow appears to be harmless in practice.
#### Remediation
We recommend changing the type of the `count` argument of `sanei_tcp_read` to `ssize_t`.
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
### Issue 9 (`GHSL-2020-084`): buffer overflow in `esci2_img`
This issue requires the user to click the "Scan" button, so it is a one-click vulnerability, rather than a zero-click vulnerability. The function [`esci2_img`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/epsonds-cmd.c#L838) has a heap buffer overflow:
```c
SANE_Status
esci2_img(struct epsonds_scanner *s, SANE_Int *length)
{
SANE_Status status = SANE_STATUS_GOOD;
SANE_Status parse_status;
unsigned int more;
ssize_t read;
*length = 0;
if (s->canceling)
return SANE_STATUS_CANCELLED;
/* request image data */
eds_send(s, "IMG x0000000", 12, &status, 64);
if (status != SANE_STATUS_GOOD) {
return status;
}
/* receive DataHeaderBlock */
memset(s->buf, 0x00, 64);
eds_recv(s, s->buf, 64, &status);
if (status != SANE_STATUS_GOOD) {
return status;
}
/* check if we need to read any image data */
more = 0;
if (!esci2_check_header("IMG ", (char *)s->buf, &more)) { <===== attacker controls value of `more`
return SANE_STATUS_IO_ERROR;
}
/* this handles eof and errors */
parse_status = esci2_parse_block((char *)s->buf + 12, 64 - 12, s, &img_cb);
/* no more data? return using the status of the esci2_parse_block
* call, which might hold other error conditions.
*/
if (!more) {
return parse_status;
}
/* ALWAYS read image data */
if (s->hw->connection == SANE_EPSONDS_NET) {
epsonds_net_request_read(s, more);
}
read = eds_recv(s, s->buf, more, &status); <===== heap buffer overflow
if (status != SANE_STATUS_GOOD) {
return status;
}
if (read != more) {
return SANE_STATUS_IO_ERROR;
}
/* handle esci2_parse_block errors */
if (parse_status != SANE_STATUS_GOOD) {
return parse_status;
}
DBG(15, "%s: read %lu bytes, status: %d\n", __func__, (unsigned long) read, status);
*length = read;
if (s->canceling) {
return SANE_STATUS_CANCELLED;
}
return SANE_STATUS_GOOD;
}
```
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
./fakescanner epson 8
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
simple-scan
```
Now, if you click the "Scan" button then you should see `simple-scan` crash with an error message like this:
```
corrupted size vs. prev_size
Aborted (core dumped)
```
#### Impact
This issue may lead to remote code execution, where "remote" means a device or computer connected to the same network as the victim. For example, in a typical office environment the malicious device would need to be somewhere inside the building.
Mitigations such as ASLR may make it difficult for an attacker to create a reliable RCE exploit for the vulnerability. The PoC that we have provided only causes libsane to crash.
#### Remediation
`esci2_img` should check that the value of `more` does not exceed the size of the buffer (`s->buf`).
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
### Issue 10: SIGFPE in `mc_setup_block_mode`
This issue requires the user to click the "Scan" button, so it is a one-click vulnerability, rather than a zero-click vulnerability. [`mc_setup_block_mode`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/magicolor.c#L1142) does some arithmetic on attacker-controlled values, which can lead to a crash due to a divide-by-zero:
```
static SANE_Status
mc_setup_block_mode (Magicolor_Scanner *s)
{
/* block_len should always be a multiple of bytes_per_line, so
* we retrieve only whole lines at once */
s->block_len = (int)(0xff00/s->scan_bytes_per_line) * s->scan_bytes_per_line; <===== divide-by-zero
s->blocks = s->data_len / s->block_len;
s->last_len = s->data_len - (s->blocks * s->block_len);
if (s->last_len>0)
s->blocks++;
DBG(5, "%s: block_len=0x%x, last_len=0x%0x, blocks=%d\n", __func__, s->block_len, s->last_len, s->blocks);
s->counter = 0;
s->bytes_read_in_line = 0;
if (s->line_buffer)
free(s->line_buffer);
s->line_buffer = malloc(s->scan_bytes_per_line);
if (s->line_buffer == NULL) {
DBG(1, "out of memory (line %d)\n", __LINE__);
return SANE_STATUS_NO_MEM;
}
DBG (5, " %s: Setup block mode - scan_bytes_per_line=%d, pixels_per_line=%d, depth=%d, data_len=%x, block_len=%x, blocks=%d, last_len=%x\n",
__func__, s->scan_bytes_per_line, s->params.pixels_per_line, s->params.depth, s->data_len, s->block_len, s->blocks, s->last_len);
return SANE_STATUS_GOOD;
}
```
The value of `s->scan_bytes_per_line` is attacker controlled. If it's zero, then the code crashes due to a divide-by-zero error.
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
sudo ./fakescanner magicolor
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
simple-scan
```
Now, if you click the "Scan" button then you should see `simple-scan` crash with an error message like this:
```
Floating point exception (core dumped)
```
#### Impact
This issue does not have any security impact, because the user has to click the "Scan" button to trigger the bug and the only consequence is that libsane will crash.
#### Remediation
[`cmd_get_scanning_parameters`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/magicolor.c#L793) should do better validation of the parameters received from the device.
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
### Issue 11: read of uninitialized data in `mc_get_device_from_identification`
The function [`mc_network_discovery_handle`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/magicolor.c#L1829) calls `mc_get_device_from_identification` on line 1915 with `device` containing uninitialized data. Ironically, this does not cause anything to go wrong due to a bug in the implementation of [`mc_get_device_from_identification`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/magicolor.c#L1523):
```c
static struct MagicolorCap *
mc_get_device_from_identification (const char*ident)
{
int n;
for (n = 0; n < NELEMS (magicolor_cap); n++) {
if (strcmp (magicolor_cap[n].model, ident) || strcmp (magicolor_cap[n].OID, ident))
return &magicolor_cap[n];
}
return NULL;
}
```
The bug is that `strcmp` returns 0 when the strings are equal, so this function always succeeds immediately on the first iteration of the loop, even when `ident` contains garbage (which it does because it's uninitialized).
We have included a proof-of-concept exploit for this vulnerability. Compile and run the PoC as follows:
```bash
g++ fakescanner.cpp -o fakescanner
sudo ./fakescanner magicolor
```
Now run `simple-scan` on a different computer that is connected to the same network (ethernet or wifi):
```bash
$ gdb simple-scan
(gdb) break mc_get_device_from_identification
(gdb) run
```
You need to run `simple-scan` in a debugger (gdb) to see this bug, because it does not cause a crash.
#### Impact
This issue does not have any security impact.
#### Remediation
Use `memset` to initialize `device` at the beginning of [`mc_network_discovery_handle`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/magicolor.c#L1829).
Check the return value of `strcmp` correctly in [`mc_get_device_from_identification`](https://gitlab.com/sane-project/backends/-/blob/1.0.27/backend/magicolor.c#L1523):
```c
static struct MagicolorCap *
mc_get_device_from_identification (const char*ident)
{
int n;
for (n = 0; n < NELEMS (magicolor_cap); n++) {
if (strcmp (magicolor_cap[n].model, ident) == 0 || strcmp (magicolor_cap[n].OID, ident) == 0)
return &magicolor_cap[n];
}
return NULL;
}
```
#### Resources
We have attached the source code for our PoC, [fakescanner.cpp](/uploads/7aa1c15de003074e2c13dc928f0a523f/fakescanner.cpp), to this report.
## Credit
These issues were discovered and reported by GHSL team member [@kevinbackhouse (Kevin Backhouse)](https://github.com/kevinbackhouse).
## Contact
You can contact the GHSL team at `securitylab@github.com`, please include the relevant GHSL IDs in any communication regarding these issues.
## Disclosure Policy
This report is subject to our [coordinated disclosure policy](https://securitylab.github.com/disclosures#policy).
issue