Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Possibly buffer overrun in controller_print() #97

Closed
technik3k opened this issue Feb 4, 2019 · 0 comments
Closed

Possibly buffer overrun in controller_print() #97

technik3k opened this issue Feb 4, 2019 · 0 comments
Assignees
@baylessj
Labels
bug Something isn't working in progress This is currently being worked on p: normal Normal priority
Milestone
3.1.6

Comments

@technik3k
Copy link

Could vasprintf() return buffer that is smaller than 16 bytes?

It is, probably, a good idea to check return value vasprintf() which is guaranteed to allocate only number of printed bytes + 1.

pros/src/devices/controller.c

Line 248 in bc0fa3e

buf[CONTROLLER_MAX_COLS] = '\0';

@baylessj baylessj mentioned this issue Feb 5, 2019
Merged
3 tasks
@baylessj baylessj self-assigned this Feb 5, 2019
@baylessj baylessj added the in progress This is currently being worked on label Feb 5, 2019
@HotelCalifornia HotelCalifornia added p: normal Normal priority bug Something isn't working labels Feb 7, 2019
@HotelCalifornia HotelCalifornia added this to the 3.1.6 milestone Feb 13, 2019
HotelCalifornia pushed a commit that referenced this issue Feb 18, 2019
@baylessj @HotelCalifornia
🐛 Prevent Buffer Overrun in Controller Printing (#98)
d3145c0 
#### 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@baylessj baylessj

Labels
bug Something isn't working in progress This is currently being worked on p: normal Normal priority
Projects
None yet
Milestone
3.1.6
Development

No branches or pull requests
3 participants
@HotelCalifornia @technik3k @baylessj