Skip to content

Commit

Permalink
Fix incorrect buffer size passed to function
Browse files Browse the repository at this point in the history
The function expects a widechar count rather than a byte count.
This is a serious error that would lead to a buffer overflow
if not for the fact that the input char buffer passed to the
function uses the same size constant as the output wchar buffer
  • Loading branch information
JFreegman committed Dec 24, 2023
1 parent a781ff8 commit ed8b401
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 11 deletions.
13 changes: 8 additions & 5 deletions src/line_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,10 @@ static int print_wrap(WINDOW *win, struct line_info *line, int max_x, int max_y)
return 0;
}

/* Converts `msg` into a widechar string and puts the result in `buf`.
/* Converts the multibyte string `msg` into a wide character string and puts
* the result in `buf`.
*
* `buf_size` is the number of wide characters that `buf` can hold.
*
* Returns the widechar width of the string.
*/
Expand All @@ -353,13 +356,13 @@ static uint16_t line_info_add_msg(wchar_t *buf, size_t buf_size, const char *msg
return 0;
}

const wint_t wc_msg_len = mbs_to_wcs_buf(buf, msg, buf_size);
const int wc_msg_len = mbs_to_wcs_buf(buf, msg, buf_size);

if (wc_msg_len > 0 && wc_msg_len < buf_size) {
buf[wc_msg_len] = L'\0';
int width = wcswidth(buf, wc_msg_len);

if (width == -1) { // the best we can do on failure is to fall back to strlen
if (width < 0 || width > UINT16_MAX) { // the best we can do on failure is to fall back to strlen
width = strlen(msg);
}

Expand Down Expand Up @@ -473,7 +476,7 @@ int line_info_add(ToxWindow *self, bool show_timestamp, const char *name1, const
break;
}

const uint16_t msg_width = line_info_add_msg(new_line->msg, sizeof(new_line->msg), frmt_msg);
const uint16_t msg_width = line_info_add_msg(new_line->msg, sizeof(new_line->msg) / sizeof(wchar_t), frmt_msg);
len += msg_width;

if (show_timestamp) {
Expand Down Expand Up @@ -853,7 +856,7 @@ void line_info_set(ToxWindow *self, uint32_t id, char *msg)

while (line) {
if (line->id == id) {
const uint16_t new_width = line_info_add_msg(line->msg, sizeof(line->msg), msg);
const uint16_t new_width = line_info_add_msg(line->msg, sizeof(line->msg) / sizeof(wchar_t), msg);
line->len = line->len - line->msg_width + new_width;
line->msg_width = new_width;
return;
Expand Down
2 changes: 0 additions & 2 deletions src/misc_tools.c
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ int wstring_is_empty(const wchar_t *string)
return string[0] == L'\0';
}

/* convert a multibyte string to a wide character string and puts in buf. */
int mbs_to_wcs_buf(wchar_t *buf, const char *string, size_t n)
{
size_t len = mbstowcs(NULL, string, 0) + 1;
Expand All @@ -283,7 +282,6 @@ int mbs_to_wcs_buf(wchar_t *buf, const char *string, size_t n)
return len;
}

/* converts wide character string into a multibyte string and puts in buf. */
int wcs_to_mbs_buf(char *buf, const wchar_t *string, size_t n)
{
size_t len = wcstombs(NULL, string, 0) + 1;
Expand Down
20 changes: 16 additions & 4 deletions src/misc_tools.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,24 @@ int wstring_is_empty(const wchar_t *string);
/* converts a multibyte string to a wide character string (must provide buffer) */
int char_to_wcs_buf(wchar_t *buf, const char *string, size_t n);

/* converts wide character string into a multibyte string and puts in buf. */
int wcs_to_mbs_buf(char *buf, const wchar_t *string, size_t n);

/* converts a multibyte string to a wide character string and puts in buf) */
/* Converts a multibyte string to a wide character string and puts in `buf`.
*
* `buf` must have room for at least `n` wide characters (wchar_t's).
*
* Return number of wide characters written on success.
* Return -1 on failure.
*/
int mbs_to_wcs_buf(wchar_t *buf, const char *string, size_t n);

/* Converts a wide character string into a multibyte string and puts in `buf`.
*
* `buf` must have room for at least `n` bytes.
*
* Return number of bytes written on success.
* Return -1 on failure.
*/
int wcs_to_mbs_buf(char *buf, const wchar_t *string, size_t n);

/* Returns 1 if connection has timed out, 0 otherwise */
int timed_out(time_t timestamp, time_t timeout);

Expand Down

0 comments on commit ed8b401

Please sign in to comment.