write-value, write-long-value and write-prepare were parsing
bytes using strtol with base '0' and restraining wtring size to
be exactly 2, forbidding to write values over 99. The string length
is no more checked, we instead check that the parsed value is in the
correct range.
---
tools/btgatt-client.c | 41 +++++++++++++----------------------------
1 file changed, 13 insertions(+), 28 deletions(-)
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 0f6a1bd..3a0ec62 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -658,7 +658,7 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
static void cmd_write_value(struct client *cli, char *cmd_str)
{
- int opt, i;
+ int opt, i, val;
char *argvbuf[516];
char **argv = argvbuf;
int argc = 1;
@@ -726,19 +726,14 @@ static void cmd_write_value(struct client *cli, char *cmd_str)
}
for (i = 1; i < argc; i++) {
- if (strlen(argv[i]) != 2) {
- printf("Invalid value byte: %s\n",
- argv[i]);
- goto done;
- }
-
- value[i-1] = strtol(argv[i], &endptr, 0);
+ val = strtol(argv[i], &endptr, 0);
if (endptr == argv[i] || *endptr != '\0'
- || errno == ERANGE) {
+ || errno == ERANGE || val < 0 || val > 255) {
printf("Invalid value byte: %s\n",
argv[i]);
goto done;
}
+ value[i-1] = val;
}
}
@@ -793,7 +788,7 @@ static void write_long_cb(bool success, bool reliable_error, uint8_t att_ecode,
static void cmd_write_long_value(struct client *cli, char *cmd_str)
{
- int opt, i;
+ int opt, i, val;
char *argvbuf[516];
char **argv = argvbuf;
int argc = 1;
@@ -865,21 +860,15 @@ static void cmd_write_long_value(struct client *cli, char *cmd_str)
}
for (i = 2; i < argc; i++) {
- if (strlen(argv[i]) != 2) {
- printf("Invalid value byte: %s\n",
- argv[i]);
- free(value);
- return;
- }
-
- value[i-2] = strtol(argv[i], &endptr, 0);
+ val = strtol(argv[i], &endptr, 0);
if (endptr == argv[i] || *endptr != '\0'
- || errno == ERANGE) {
+ || errno == ERANGE || val < 0 || val > 255) {
printf("Invalid value byte: %s\n",
argv[i]);
free(value);
return;
}
+ value[i-2] = val;
}
}
@@ -909,7 +898,7 @@ static struct option write_prepare_options[] = {
static void cmd_write_prepare(struct client *cli, char *cmd_str)
{
- int opt, i;
+ int opt, i, val;
char *argvbuf[516];
char **argv = argvbuf;
int argc = 0;
@@ -1002,18 +991,14 @@ static void cmd_write_prepare(struct client *cli, char *cmd_str)
}
for (i = 2; i < argc; i++) {
- if (strlen(argv[i]) != 2) {
- printf("Invalid value byte: %s\n", argv[i]);
- free(value);
- return;
- }
-
- value[i-2] = strtol(argv[i], &endptr, 0);
- if (endptr == argv[i] || *endptr != '\0' || errno == ERANGE) {
+ val = strtol(argv[i], &endptr, 0);
+ if (endptr == argv[i] || *endptr != '\0' || errno == ERANGE
+ || val < 0 || val > 255) {
printf("Invalid value byte: %s\n", argv[i]);
free(value);
return;
}
+ value[i-2] = val;
}
done:
--
2.1.4
Hi Maxime,
On Wed, Feb 24, 2016 at 10:35 AM, Chevallier Maxime
<[email protected]> wrote:
> write-value, write-long-value and write-prepare were parsing
> bytes using strtol with base '0' and restraining wtring size to
> be exactly 2, forbidding to write values over 99. The string length
> is no more checked, we instead check that the parsed value is in the
> correct range.
There seems to be a space in the subject in between ':' please remove that.
> ---
> tools/btgatt-client.c | 41 +++++++++++++----------------------------
> 1 file changed, 13 insertions(+), 28 deletions(-)
>
> diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
> index 0f6a1bd..3a0ec62 100644
> --- a/tools/btgatt-client.c
> +++ b/tools/btgatt-client.c
> @@ -658,7 +658,7 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
>
> static void cmd_write_value(struct client *cli, char *cmd_str)
> {
> - int opt, i;
> + int opt, i, val;
> char *argvbuf[516];
> char **argv = argvbuf;
> int argc = 1;
> @@ -726,19 +726,14 @@ static void cmd_write_value(struct client *cli, char *cmd_str)
> }
>
> for (i = 1; i < argc; i++) {
> - if (strlen(argv[i]) != 2) {
> - printf("Invalid value byte: %s\n",
> - argv[i]);
> - goto done;
> - }
> -
> - value[i-1] = strtol(argv[i], &endptr, 0);
> + val = strtol(argv[i], &endptr, 0);
> if (endptr == argv[i] || *endptr != '\0'
> - || errno == ERANGE) {
> + || errno == ERANGE || val < 0 || val > 255) {
> printf("Invalid value byte: %s\n",
> argv[i]);
> goto done;
> }
> + value[i-1] = val;
> }
> }
>
> @@ -793,7 +788,7 @@ static void write_long_cb(bool success, bool reliable_error, uint8_t att_ecode,
>
> static void cmd_write_long_value(struct client *cli, char *cmd_str)
> {
> - int opt, i;
> + int opt, i, val;
> char *argvbuf[516];
> char **argv = argvbuf;
> int argc = 1;
> @@ -865,21 +860,15 @@ static void cmd_write_long_value(struct client *cli, char *cmd_str)
> }
>
> for (i = 2; i < argc; i++) {
> - if (strlen(argv[i]) != 2) {
> - printf("Invalid value byte: %s\n",
> - argv[i]);
> - free(value);
> - return;
> - }
> -
> - value[i-2] = strtol(argv[i], &endptr, 0);
> + val = strtol(argv[i], &endptr, 0);
> if (endptr == argv[i] || *endptr != '\0'
> - || errno == ERANGE) {
> + || errno == ERANGE || val < 0 || val > 255) {
> printf("Invalid value byte: %s\n",
> argv[i]);
> free(value);
> return;
> }
> + value[i-2] = val;
> }
> }
>
> @@ -909,7 +898,7 @@ static struct option write_prepare_options[] = {
>
> static void cmd_write_prepare(struct client *cli, char *cmd_str)
> {
> - int opt, i;
> + int opt, i, val;
> char *argvbuf[516];
> char **argv = argvbuf;
> int argc = 0;
> @@ -1002,18 +991,14 @@ static void cmd_write_prepare(struct client *cli, char *cmd_str)
> }
>
> for (i = 2; i < argc; i++) {
> - if (strlen(argv[i]) != 2) {
> - printf("Invalid value byte: %s\n", argv[i]);
> - free(value);
> - return;
> - }
> -
> - value[i-2] = strtol(argv[i], &endptr, 0);
> - if (endptr == argv[i] || *endptr != '\0' || errno == ERANGE) {
> + val = strtol(argv[i], &endptr, 0);
> + if (endptr == argv[i] || *endptr != '\0' || errno == ERANGE
> + || val < 0 || val > 255) {
> printf("Invalid value byte: %s\n", argv[i]);
> free(value);
> return;
> }
> + value[i-2] = val;
> }
>
> done:
> --
> 2.1.4
Overall the patch looks pretty good but please fix the following:
WARNING:LONG_LINE: line over 80 characters
#28: FILE: tools/btgatt-client.c:731:
+ || errno == ERANGE || val < 0 || val > 255) {
WARNING:LONG_LINE: line over 80 characters
#61: FILE: tools/btgatt-client.c:865:
+ || errno == ERANGE || val < 0 || val > 255) {
WARNING:LONG_LINE: line over 80 characters
#94: FILE: tools/btgatt-client.c:996:
+ || val < 0 || val > 255) {
--
Luiz Augusto von Dentz