search

sysprog21 / mado

A window system for resource-constrained devices
MIT License
34 stars 12 forks source link

Change sprintf to snprintf #24

Closed Damien-Chen closed 1 month ago

Damien-Chen commented 1 month ago

sprintf does not perform any bounds checking, which could potentially lead to occurence of buffer overflow and cause undefined behavior.

snprintf require to specify the size of buffer so that it is able to prevent undefined behavior. Adding sizeof(v) + 1 to include null terminator.

Reference: https://www.gnu.org/software/libc/manual/html_node/Formatted-Output-Functions.html

jserv commented 1 month ago

snprintf require to specify the size of buffer so that it is able to prevent undefined behavior. Adding sizeof(v) + 1 to include null terminator.

In the calculator application, do you think it is susceptible to a buffer overrun? If so, provide a minimal reproducible example to demonstrate the need for the proposed change.

Damien-Chen commented 1 month ago

Yes you are right, current char array is 20-byte long, after investigation I think it is suffcient to store int type integer.

Damien-Chen commented 1 month ago

However, there is one strange problem that once the calculator reach the limit of its length, next time I press a button of a number, the number in the calculator shows will randomly change to other number.

jserv commented 1 month ago

However, there is one strange problem that once the calculator reach the limit of its length, next time I press a button of a number, the number in the calculator shows will randomly change to other number.

Write down the steps to reproduce the issue, along with your analysis, and submit it in another GitHub issue.