Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 78f3f41

Browse filesBrowse files
aentingeraentinger
authored
Merge pull request #131 from arduino/fix-buf-size-float
Bugfix: Small buffer size causes buffer overrun when invoking String::Ctor with large float/double value.
2 parents 4f19438 + 3c76ef2 commit 78f3f41
Copy full SHA for 78f3f41

File tree

3 files changed

+54
-11
lines changed
Filter options

3 files changed

+54
-11
lines changed

‎api/String.cpp

Copy file name to clipboardExpand all lines: api/String.cpp
+18-4Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,24 @@
2020
*/
2121

2222
#include "String.h"
23+
#include "Common.h"
2324
#include "itoa.h"
2425
#include "deprecated-avr-comp/avr/dtostrf.h"
2526

27+
#include <float.h>
28+
29+
namespace arduino {
30+
2631
/*********************************************/
27-
/* Constructors */
32+
/* Static Member Initialisation */
2833
/*********************************************/
2934

30-
namespace arduino {
35+
size_t const String::FLT_MAX_DECIMAL_PLACES;
36+
size_t const String::DBL_MAX_DECIMAL_PLACES;
37+
38+
/*********************************************/
39+
/* Constructors */
40+
/*********************************************/
3141

3242
String::String(const char *cstr)
3343
{
@@ -111,15 +121,19 @@ String::String(unsigned long value, unsigned char base)
111121

112122
String::String(float value, unsigned char decimalPlaces)
113123
{
124+
static size_t const FLOAT_BUF_SIZE = FLT_MAX_10_EXP + FLT_MAX_DECIMAL_PLACES + 1 /* '-' */ + 1 /* '.' */ + 1 /* '\0' */;
114125
init();
115-
char buf[33];
126+
char buf[FLOAT_BUF_SIZE];
127+
decimalPlaces = min(decimalPlaces, FLT_MAX_DECIMAL_PLACES);
116128
*this = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf);
117129
}
118130

119131
String::String(double value, unsigned char decimalPlaces)
120132
{
133+
static size_t const DOUBLE_BUF_SIZE = DBL_MAX_10_EXP + DBL_MAX_DECIMAL_PLACES + 1 /* '-' */ + 1 /* '.' */ + 1 /* '\0' */;
121134
init();
122-
char buf[33];
135+
char buf[DOUBLE_BUF_SIZE];
136+
decimalPlaces = min(decimalPlaces, DBL_MAX_DECIMAL_PLACES);
123137
*this = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf);
124138
}
125139

‎api/String.h

Copy file name to clipboardExpand all lines: api/String.h
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ class String
5858
typedef void (String::*StringIfHelperType)() const;
5959
void StringIfHelper() const {}
6060

61+
static size_t const FLT_MAX_DECIMAL_PLACES = 10;
62+
static size_t const DBL_MAX_DECIMAL_PLACES = FLT_MAX_DECIMAL_PLACES;
63+
6164
public:
6265
// constructors
6366
// creates a copy of the initial value.

‎test/src/String/test_String.cpp

Copy file name to clipboardExpand all lines: test/src/String/test_String.cpp
+33-7Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
* INCLUDE
77
**************************************************************************************/
88

9+
#include <float.h>
10+
911
#include <catch.hpp>
1012

1113
#include <String.h>
@@ -80,16 +82,40 @@ TEST_CASE ("Testing String(unsigned long, unsigned char base = 10) constructor()
8082

8183
TEST_CASE ("Testing String(float, unsigned char decimalPlaces = 2) constructor()", "[String-Ctor-10]")
8284
{
83-
float const val = 1.234f;
84-
arduino::String str(val);
85-
REQUIRE(strcmp(str.c_str(), "1.23") == 0);
85+
WHEN ("String::String (some float value)")
86+
{
87+
arduino::String str(1.234f);
88+
REQUIRE(strcmp(str.c_str(), "1.23") == 0);
89+
}
90+
WHEN ("String::String (FLT_MAX)")
91+
{
92+
arduino::String str(FLT_MAX);
93+
REQUIRE(strcmp(str.c_str(), "340282346638528859811704183484516925440.00") == 0);
94+
}
95+
WHEN ("String::String (-FLT_MAX)")
96+
{
97+
arduino::String str(-FLT_MAX);
98+
REQUIRE(strcmp(str.c_str(), "-340282346638528859811704183484516925440.00") == 0);
99+
}
86100
}
87101

88102
TEST_CASE ("Testing String(double, unsigned char decimalPlaces = 2) constructor()", "[String-Ctor-11]")
89103
{
90-
double const val = 5.678;
91-
arduino::String str(val);
92-
REQUIRE(strcmp(str.c_str(), "5.68") == 0);
104+
WHEN ("String::String (some double value)")
105+
{
106+
arduino::String str(5.678);
107+
REQUIRE(strcmp(str.c_str(), "5.68") == 0);
108+
}
109+
WHEN ("String::String (DBL_MAX)")
110+
{
111+
arduino::String str(DBL_MAX);
112+
REQUIRE(strcmp(str.c_str(), "179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.00") == 0);
113+
}
114+
WHEN ("String::String (-DBL_MAX)")
115+
{
116+
arduino::String str(-DBL_MAX);
117+
REQUIRE(strcmp(str.c_str(), "-179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.00") == 0);
118+
}
93119
}
94120

95121
TEST_CASE ("Testing String(const __FlashStringHelper) constructor() with invalid buffer", "[String-Ctor-12]")
@@ -131,4 +157,4 @@ TEST_CASE ("Testing String(String &&) with move(String &rhs) from larger to smal
131157
arduino::String str1("Arduino");
132158
str = static_cast<arduino::String&&>(str1);
133159
REQUIRE(str1.compareTo("Arduino") == 0);
134-
}
160+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.