[PATCH] simutil: Fill unused part of AID with FFs

[PATCH] simutil: Fill unused part of AID with FFs

[Slava Monich]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

Correct handling of short AIDs will take more than that, but
leaving part of the array uninitialized is wrong in any case.
---
 src/simutil.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/simutil.c b/src/simutil.c
index 5d2aa6a2..e648c918 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -1588,6 +1588,7 @@ GSList *sim_parse_app_template_entries(const unsigned char *buffer, int len)
 			goto error;
 
 		memcpy(app.aid, aid, app.aid_len);
+		memset(app.aid + app.aid_len, 0xff, 16 - app.aid_len);
 
 		app.type = (app.aid[5] << 8) | app.aid[6];
 
-- 
2.25.1

Re: [PATCH] simutil: Fill unused part of AID with FFs

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

Hi Slava,

On 4/29/21 11:09 AM, Slava Monich wrote:
> Correct handling of short AIDs will take more than that, but
> leaving part of the array uninitialized is wrong in any case.
> ---
>   src/simutil.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/src/simutil.c b/src/simutil.c
> index 5d2aa6a2..e648c918 100644
> --- a/src/simutil.c
> +++ b/src/simutil.c
> @@ -1588,6 +1588,7 @@ GSList *sim_parse_app_template_entries(const unsigned char *buffer, int len)
>   			goto error;
>   
>   		memcpy(app.aid, aid, app.aid_len);
> +		memset(app.aid + app.aid_len, 0xff, 16 - app.aid_len);

Would it not be easier to fix sim-auth to take aid_len into account instead of 
hard-coding 16?  It seems like sim_auth_register is the only one affected, but I 
didn't look deeply.

>   
>   		app.type = (app.aid[5] << 8) | app.aid[6];
>   
> 

Regards,
-Denis

Re: [PATCH] simutil: Fill unused part of AID with FFs

[Slava Monich]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On 30.4.2021 17.54, Denis Kenzior wrote:
> Hi Slava,
>
> On 4/29/21 11:09 AM, Slava Monich wrote:
>> Correct handling of short AIDs will take more than that, but
>> leaving part of the array uninitialized is wrong in any case.
>> ---
>>   src/simutil.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/simutil.c b/src/simutil.c
>> index 5d2aa6a2..e648c918 100644
>> --- a/src/simutil.c
>> +++ b/src/simutil.c
>> @@ -1588,6 +1588,7 @@ GSList *sim_parse_app_template_entries(const 
>> unsigned char *buffer, int len)
>>               goto error;
>>             memcpy(app.aid, aid, app.aid_len);
>> +        memset(app.aid + app.aid_len, 0xff, 16 - app.aid_len);
>
> Would it not be easier to fix sim-auth to take aid_len into account 
> instead of hard-coding 16?  It seems like sim_auth_register is the 
> only one affected, but I didn't look deeply.


AFAICT it's not that trivial but feel free to disregard this patch - it 
doesn't make much sense to fix it half-way.


Rebards,

-Slava