strutils: fix buffer overflow in formatBiggestFloat

nim-works / nimskull

An in development statically typed systems programming language; with sustainability at its core. We, the community of users, maintain it.

https://nim-works.github.io/nimskull/index.html

Other

275 stars 39 forks source link

strutils: fix buffer overflow in formatBiggestFloat #1423

Closed alaviss closed 1 month ago

alaviss commented 1 month ago

Summary

Fixed a potential buffer overflow in struts.formatBiggestFloat.

Details

Previously, strutils.formatBiggestFloat was implemented using sprintf and as such came with a risk of buffer overflow. This was observed via tests in disruptek/insideout.

Replaced sprintf with snprintf.
Remove the 2 KiB temporary buffer in favor of directly outputting to the target buffer.
Added extra error checking code for errors from snprintf.

alaviss commented 1 month ago

Don't suppose there is an easy enough reproduction for a test, or do you think it's too implementation specific?

It's too implementation specific and won't be of any help if we move to a native impl at one point.

saem commented 1 month ago

/merge

(updated the PR body slightly)

github-actions[bot] commented 1 month ago

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:

## Notes for Reviewers * While Dragonbox is bundled with the project, it is not suitable for generating fixed-precision numbers. As such the current `printf`-based method continues to be used and migration to a native algorithm will have to be done later.