Skip to content

Commit

Permalink
🐛 Prevent Buffer Overrun in Controller Printing (#98)
Browse files Browse the repository at this point in the history
#### Summary:
This moves to more secure string operations for the `controller_set_text` and `controller_print` functions. Previously, `controller_print` would overrun the buffer when printing a string smaller than the maximum allowable length, and a string longer than CONTROLLER_MAX_COLS could be written into the smaller destination buffer in `controller_set_text`.

#### Motivation:
This will prevent buffer overrun and its associated issues in the controller printing functions.

##### References (optional):
Closes #97.

#### Test Plan:
- [x] Compiles
- [x] Call `controller_set_text` with a string longer than `CONTROLLER_MAX_COLS`
- [x] Call `controller_print` with a string that is shorter than `CONTROLLER_MAX_COLS`

#### Commits:
* Improve controller printing safety

* fix pointer reference

* use strndup instead of strlcpy

strlcpy is nonstandard, even by our standards (lol)
  • Loading branch information
baylessj authored and HotelCalifornia committed Feb 18, 2019
1 parent 47b06f0 commit d3145c0
Showing 1 changed file with 4 additions and 11 deletions.
15 changes: 4 additions & 11 deletions src/devices/controller.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

#define _GNU_SOURCE
// NOTE: this would normally be in the C file, but it won't compile that way
#include <stdio.h> // vasprintf (GNU extension)
#undef _GNU_SOURCE
#include <stdio.h>

#include "kapi.h"
#include "v5_api.h"
Expand Down Expand Up @@ -207,9 +204,7 @@ int32_t controller_set_text(controller_id_e_t id, uint8_t line, uint8_t col, con
else
col++;

char* buf = (char*)malloc(CONTROLLER_MAX_COLS + 1);
strcpy(buf, str);
buf[CONTROLLER_MAX_COLS] = '\0';
char* buf = strndup(str, CONTROLLER_MAX_COLS + 1);

vexControllerTextSet(id, line, col, buf);
free(buf);
Expand Down Expand Up @@ -242,10 +237,8 @@ int32_t controller_print(controller_id_e_t id, uint8_t line, uint8_t col, const

va_list args;
va_start(args, fmt);
char* buf;
vasprintf(&buf, fmt, args);

buf[CONTROLLER_MAX_COLS] = '\0';
char* buf = (char*)malloc(CONTROLLER_MAX_COLS + 1);
vsnprintf(buf, CONTROLLER_MAX_COLS + 1, fmt, args);

vexControllerTextSet(id, line, col, buf);
free(buf);
Expand Down

0 comments on commit d3145c0

Please sign in to comment.