2013-11-01 12:20:05

by Ravi kumar Veeramally

Subject: [PATCH] android/client: Fix pin reply call when pin argument missing in cmd

If condition checks number of arguments properly but still executes
EXEC command. It should display error message and return.
---
android/client/if-bt.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index 3d97458..2c3cc4a 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -719,12 +719,15 @@ static void pin_reply_p(int argc, const char **argv)
RETURN_IF_NULL(if_bluetooth);
VERIFY_ADDR_ARG(2, &addr);

- if (argc > 3) {
- accept = 1;
- pin_len = strlen(argv[3]);
- memcpy(pin.pin, argv[3], pin_len);
+ if (argc != 4) {
+ haltest_error("Usage <address> <pin>\n");
+ return;
}

+ accept = 1;
+ pin_len = strlen(argv[3]);
+ memcpy(pin.pin, argv[3], pin_len);
+
EXEC(if_bluetooth->pin_reply, &addr, accept, pin_len, &pin);
}

--
1.8.1.2

2013-11-04 08:40:41

by Jerzy Kasenberg

Subject: Re: [PATCH] android/client: Fix pin reply call when pin argument missing in cmd

Hi Ravi,

On 1 November 2013 14:11, Johan Hedberg <[email protected]> wrote:
> Hi Ravi,
>
> On Fri, Nov 01, 2013, Ravi kumar Veeramally wrote:
>> If condition checks number of arguments properly but still executes
>> EXEC command. It should display error message and return.
>> ---
>> android/client/if-bt.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/android/client/if-bt.c b/android/client/if-bt.c
>> index 3d97458..2c3cc4a 100644
>> --- a/android/client/if-bt.c
>> +++ b/android/client/if-bt.c
>> @@ -719,12 +719,15 @@ static void pin_reply_p(int argc, const char **argv)
>> RETURN_IF_NULL(if_bluetooth);
>> VERIFY_ADDR_ARG(2, &addr);
>>
>> - if (argc > 3) {
>> - accept = 1;
>> - pin_len = strlen(argv[3]);
>> - memcpy(pin.pin, argv[3], pin_len);
>> + if (argc != 4) {
>> + haltest_error("Usage <address> <pin>\n");
>> + return;
>> }
>>
>> + accept = 1;
>> + pin_len = strlen(argv[3]);
>> + memcpy(pin.pin, argv[3], pin_len);
>> +
>> EXEC(if_bluetooth->pin_reply, &addr, accept, pin_len, &pin);
>
> It seems to me that the original code might have been trying to do (if
> it hadn't been broken like it now is) a negative pin reply in the case
> that no parameter was given. At least I don't see any other code trying
> to call if_bluetooth->pin_reply with accept=1 in haltest. So I'd
> consider fixing that instead.

Idea here was: no pin given set accept to 0, so user would not need to
type accept argument at all.
What is missing though, is initialization of accept to 0.

--
Jerzy

2013-11-01 13:11:41

by Johan Hedberg

Subject: Re: [PATCH] android/client: Fix pin reply call when pin argument missing in cmd

Hi Ravi,

On Fri, Nov 01, 2013, Ravi kumar Veeramally wrote:
> If condition checks number of arguments properly but still executes
> EXEC command. It should display error message and return.
> ---
> android/client/if-bt.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/android/client/if-bt.c b/android/client/if-bt.c
> index 3d97458..2c3cc4a 100644
> --- a/android/client/if-bt.c
> +++ b/android/client/if-bt.c
> @@ -719,12 +719,15 @@ static void pin_reply_p(int argc, const char **argv)
> RETURN_IF_NULL(if_bluetooth);
> VERIFY_ADDR_ARG(2, &addr);
>
> - if (argc > 3) {
> - accept = 1;
> - pin_len = strlen(argv[3]);
> - memcpy(pin.pin, argv[3], pin_len);
> + if (argc != 4) {
> + haltest_error("Usage <address> <pin>\n");
> + return;
> }
>
> + accept = 1;
> + pin_len = strlen(argv[3]);
> + memcpy(pin.pin, argv[3], pin_len);
> +
> EXEC(if_bluetooth->pin_reply, &addr, accept, pin_len, &pin);

It seems to me that the original code might have been trying to do (if
it hadn't been broken like it now is) a negative pin reply in the case
that no parameter was given. At least I don't see any other code trying
to call if_bluetooth->pin_reply with accept=1 in haltest. So I'd
consider fixing that instead.

Johan