buffer overflow in card-myeid.c

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

buffer overflow in card-myeid.c

Peter Popovec
Hi,

I found this in opensc-0.14.0:
 
0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:769:sc_card_ctl: called
0x7fdf91066700 10:53:44.400 [pkcs15-init] card-myeid.c:1189:myeid_card_ctl:
called
0x7fdf91066700 10:53:44.400 [pkcs15-init] card-myeid.c:1215:myeid_card_ctl:
returning with: -1408 (Not supported)
0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:776:sc_card_ctl:
card_ctl(4) not supported
0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:414:sc_create_file: called;
type=2, path=3f005015, size=5000
0x7fdf91066700 10:53:44.400 [pkcs15-init]
card-myeid.c:439:myeid_create_file: called
0x7fdf91066700 10:53:44.400 [pkcs15-init]
card-myeid.c:311:encode_file_structure: called
*** buffer overflow detected ***: pkcs15-init terminated
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x731ff)[0x7fdf8ff861ff]
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x7fdf900094c7]
/lib/x86_64-linux-gnu/libc.so.6(+0xf46e0)[0x7fdf900076e0]
/usr/lib/x86_64-linux-gnu/libopensc.so.3(+0x98026)[0x7fdf90775026]
/usr/lib/x86_64-linux-gnu/libopensc.so.3(sc_create_file+0xaa)[0x7fdf9070061a]
/usr/lib/x86_64-linux-gnu/libopensc.so.3(sc_pkcs15init_create_file+0x113)[0x7fdf907be063]

Analysis:

Function myeid_create_file() allocates buffer of 32 bytes. Then it  calls
encode_file_structure() function. This function allocates buffer of 42 bytes.

First buffer overflow can be hit if filetype is SC_FILE_TYPE_DF. Function
encode_file_structure() needs 0x19 + file->namelen + 2 bytes in buffer =
max 45 bytes for file->namelen = 16. Buffer is three bytes smaller ..   

Second buffer overflow can be hit at the end of encode_file_structure().
Function  memcpy(out, buf, *outlen) is called at worst case with outlen=45.
Because the space in destination string is only 32 bytes (allocated by
myeid_create_file() function), buffer overflow is generated.


Please, use 45 bytes for these buffers:


diff --git a/src/libopensc/card-myeid.c b/src/libopensc/card-myeid.c
index 2e5f9c6..babae88 100644
--- a/src/libopensc/card-myeid.c
+++ b/src/libopensc/card-myeid.c
@@ -305,7 +305,7 @@ static int encode_file_structure(sc_card_t *card, const
sc_file_t *file,
                u8 *out, size_t *outlen)
 {
        const sc_acl_entry_t *read, *update, *delete, *generate;
-       u8 buf[40];
+       u8 buf[45];
        int i;
 
        LOG_FUNC_CALLED(card->ctx);
@@ -432,7 +432,7 @@ static int encode_file_structure(sc_card_t *card, const
sc_file_t *file,
 static int myeid_create_file(struct sc_card *card, struct sc_file *file)
 {
        sc_apdu_t apdu;
-       u8 sbuf[32];  
+       u8 sbuf[45];  
         size_t buflen;
        int r;
 
 There is another error encode_file_structure() function:

       case SC_FILE_TYPE_DF:
                buf[8] = 0x38;
                if(file->namelen > 0 && file->namelen <= 16)
                {
                        buf[25] = 0x84;
                        buf[26] = (u8)file->namelen;

                        for(i=0;i < file->namelen;i++)
                                buf[i + 26] = file->name[i];

                        buf[1] = 0x19 + file->namelen + 2;
                }
                break;


First, buf[26] is set to actual namelen:

buf[26] = (u8)file->namelen;

Then this byte will be overwriten by first character of DF name:

buf[i + 26] = file->name[i];

Patch fo tix this issue:

diff --git a/src/libopensc/card-myeid.c b/src/libopensc/card-myeid.c
index e8c28ec..53f904b 100644
--- a/src/libopensc/card-myeid.c
+++ b/src/libopensc/card-myeid.c
@@ -453,7 +453,7 @@ static int encode_file_structure(sc_card_t *card, const
sc_file_t *file,
                        buf[26] = (u8)file->namelen;
 
                        for(i=0;i < file->namelen;i++)
-                               buf[i + 26] = file->name[i];
+                               buf[i + 27] = file->name[i];
 
                        buf[1] = 0x19 + file->namelen + 2;
                }



------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: buffer overflow in card-myeid.c

Frank Morgner
Thanks for the report, the fix looks OK. Could you create a pull request
on Github?

On Monday, March 30 at 09:55AM, Peter Popovec wrote:

> Hi,
>
> I found this in opensc-0.14.0:
>
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:769:sc_card_ctl: called
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card-myeid.c:1189:myeid_card_ctl:
> called
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card-myeid.c:1215:myeid_card_ctl:
> returning with: -1408 (Not supported)
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:776:sc_card_ctl:
> card_ctl(4) not supported
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:414:sc_create_file: called;
> type=2, path=3f005015, size=5000
> 0x7fdf91066700 10:53:44.400 [pkcs15-init]
> card-myeid.c:439:myeid_create_file: called
> 0x7fdf91066700 10:53:44.400 [pkcs15-init]
> card-myeid.c:311:encode_file_structure: called
> *** buffer overflow detected ***: pkcs15-init terminated
> ======= Backtrace: =========
> /lib/x86_64-linux-gnu/libc.so.6(+0x731ff)[0x7fdf8ff861ff]
> /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x7fdf900094c7]
> /lib/x86_64-linux-gnu/libc.so.6(+0xf46e0)[0x7fdf900076e0]
> /usr/lib/x86_64-linux-gnu/libopensc.so.3(+0x98026)[0x7fdf90775026]
> /usr/lib/x86_64-linux-gnu/libopensc.so.3(sc_create_file+0xaa)[0x7fdf9070061a]
> /usr/lib/x86_64-linux-gnu/libopensc.so.3(sc_pkcs15init_create_file+0x113)[0x7fdf907be063]
>
> Analysis:
>
> Function myeid_create_file() allocates buffer of 32 bytes. Then it  calls
> encode_file_structure() function. This function allocates buffer of 42
> bytes.
>
> First buffer overflow can be hit if filetype is SC_FILE_TYPE_DF. Function
> encode_file_structure() needs 0x19 + file->namelen + 2 bytes in buffer =
> max 45 bytes for file->namelen = 16. Buffer is three bytes smaller ..
>
> Second buffer overflow can be hit at the end of encode_file_structure().
> Function  memcpy(out, buf, *outlen) is called at worst case with outlen=45.
> Because the space in destination string is only 32 bytes (allocated by
> myeid_create_file() function), buffer overflow is generated.
>
>
> Please, use 45 bytes for these buffers:
>
>
> diff --git a/src/libopensc/card-myeid.c b/src/libopensc/card-myeid.c
> index 2e5f9c6..babae88 100644
> --- a/src/libopensc/card-myeid.c
> +++ b/src/libopensc/card-myeid.c
> @@ -305,7 +305,7 @@ static int encode_file_structure(sc_card_t *card, const
> sc_file_t *file,
>                 u8 *out, size_t *outlen)
>  {
>         const sc_acl_entry_t *read, *update, *delete, *generate;
> -       u8 buf[40];
> +       u8 buf[45];
>         int i;
>
>         LOG_FUNC_CALLED(card->ctx);
> @@ -432,7 +432,7 @@ static int encode_file_structure(sc_card_t *card, const
> sc_file_t *file,
>  static int myeid_create_file(struct sc_card *card, struct sc_file *file)
>  {
>         sc_apdu_t apdu;
> -       u8 sbuf[32];
> +       u8 sbuf[45];
>          size_t buflen;
>         int r;
>
>  There is another error encode_file_structure() function:
>
>        case SC_FILE_TYPE_DF:
>                 buf[8] = 0x38;
>                 if(file->namelen > 0 && file->namelen <= 16)
>                 {
>                         buf[25] = 0x84;
>                         buf[26] = (u8)file->namelen;
>
>                         for(i=0;i < file->namelen;i++)
>                                 buf[i + 26] = file->name[i];
>
>                         buf[1] = 0x19 + file->namelen + 2;
>                 }
>                 break;
>
>
> First, buf[26] is set to actual namelen:
>
> buf[26] = (u8)file->namelen;
>
> Then this byte will be overwriten by first character of DF name:
>
> buf[i + 26] = file->name[i];
>
> Patch fo tix this issue:
>
> diff --git a/src/libopensc/card-myeid.c b/src/libopensc/card-myeid.c
> index e8c28ec..53f904b 100644
> --- a/src/libopensc/card-myeid.c
> +++ b/src/libopensc/card-myeid.c
> @@ -453,7 +453,7 @@ static int encode_file_structure(sc_card_t *card, const
> sc_file_t *file,
>                         buf[26] = (u8)file->namelen;
>
>                         for(i=0;i < file->namelen;i++)
> -                               buf[i + 26] = file->name[i];
> +                               buf[i + 27] = file->name[i];
>
>                         buf[1] = 0x19 + file->namelen + 2;
>                 }

> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming The Go Parallel Website, sponsored
> by Intel and developed in partnership with Slashdot Media, is your hub for all
> things parallel software development, from weekly thought leadership blogs to
> news, videos, case studies, tutorials and more. Take a look and join the
> conversation now. http://goparallel.sourceforge.net/
> _______________________________________________
> Opensc-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/opensc-devel

--
Frank Morgner

Virtual Smart Card Architecture http://vsmartcard.sourceforge.net
OpenPACE                        http://openpace.sourceforge.net
IFD Handler for libnfc Devices  http://sourceforge.net/projects/ifdnfc

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel

attachment0 (985 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: buffer overflow in card-myeid.c

Peter Popovec
Hi,

I do not have a github account and I do not wish to register as a developer on github.


On Tue, Mar 31, 2015 at 2:12 PM, Frank Morgner <[hidden email]> wrote:
Thanks for the report, the fix looks OK. Could you create a pull request
on Github?

On Monday, March 30 at 09:55AM, Peter Popovec wrote:
> Hi,
>
> I found this in opensc-0.14.0:
>
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:769:sc_card_ctl: called
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card-myeid.c:1189:myeid_card_ctl:
> called
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card-myeid.c:1215:myeid_card_ctl:
> returning with: -1408 (Not supported)
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:776:sc_card_ctl:
> card_ctl(4) not supported
> 0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:414:sc_create_file: called;
> type=2, path=3f005015, size=5000
> 0x7fdf91066700 10:53:44.400 [pkcs15-init]
> card-myeid.c:439:myeid_create_file: called
> 0x7fdf91066700 10:53:44.400 [pkcs15-init]
> card-myeid.c:311:encode_file_structure: called
> *** buffer overflow detected ***: pkcs15-init terminated
> ======= Backtrace: =========
> /lib/x86_64-linux-gnu/libc.so.6(+0x731ff)[0x7fdf8ff861ff]
> /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x7fdf900094c7]
> /lib/x86_64-linux-gnu/libc.so.6(+0xf46e0)[0x7fdf900076e0]
> /usr/lib/x86_64-linux-gnu/libopensc.so.3(+0x98026)[0x7fdf90775026]
> /usr/lib/x86_64-linux-gnu/libopensc.so.3(sc_create_file+0xaa)[0x7fdf9070061a]
> /usr/lib/x86_64-linux-gnu/libopensc.so.3(sc_pkcs15init_create_file+0x113)[0x7fdf907be063]
>
> Analysis:
>
> Function myeid_create_file() allocates buffer of 32 bytes. Then it  calls
> encode_file_structure() function. This function allocates buffer of 42
> bytes.
>
> First buffer overflow can be hit if filetype is SC_FILE_TYPE_DF. Function
> encode_file_structure() needs 0x19 + file->namelen + 2 bytes in buffer =
> max 45 bytes for file->namelen = 16. Buffer is three bytes smaller ..
>
> Second buffer overflow can be hit at the end of encode_file_structure().
> Function  memcpy(out, buf, *outlen) is called at worst case with outlen=45.
> Because the space in destination string is only 32 bytes (allocated by
> myeid_create_file() function), buffer overflow is generated.
>
>
> Please, use 45 bytes for these buffers:
>
>
> diff --git a/src/libopensc/card-myeid.c b/src/libopensc/card-myeid.c
> index 2e5f9c6..babae88 100644
> --- a/src/libopensc/card-myeid.c
> +++ b/src/libopensc/card-myeid.c
> @@ -305,7 +305,7 @@ static int encode_file_structure(sc_card_t *card, const
> sc_file_t *file,
>                 u8 *out, size_t *outlen)
>  {
>         const sc_acl_entry_t *read, *update, *delete, *generate;
> -       u8 buf[40];
> +       u8 buf[45];
>         int i;
>
>         LOG_FUNC_CALLED(card->ctx);
> @@ -432,7 +432,7 @@ static int encode_file_structure(sc_card_t *card, const
> sc_file_t *file,
>  static int myeid_create_file(struct sc_card *card, struct sc_file *file)
>  {
>         sc_apdu_t apdu;
> -       u8 sbuf[32];
> +       u8 sbuf[45];
>          size_t buflen;
>         int r;
>
>  There is another error encode_file_structure() function:
>
>        case SC_FILE_TYPE_DF:
>                 buf[8] = 0x38;
>                 if(file->namelen > 0 && file->namelen <= 16)
>                 {
>                         buf[25] = 0x84;
>                         buf[26] = (u8)file->namelen;
>
>                         for(i=0;i < file->namelen;i++)
>                                 buf[i + 26] = file->name[i];
>
>                         buf[1] = 0x19 + file->namelen + 2;
>                 }
>                 break;
>
>
> First, buf[26] is set to actual namelen:
>
> buf[26] = (u8)file->namelen;
>
> Then this byte will be overwriten by first character of DF name:
>
> buf[i + 26] = file->name[i];
>
> Patch fo tix this issue:
>
> diff --git a/src/libopensc/card-myeid.c b/src/libopensc/card-myeid.c
> index e8c28ec..53f904b 100644
> --- a/src/libopensc/card-myeid.c
> +++ b/src/libopensc/card-myeid.c
> @@ -453,7 +453,7 @@ static int encode_file_structure(sc_card_t *card, const
> sc_file_t *file,
>                         buf[26] = (u8)file->namelen;
>
>                         for(i=0;i < file->namelen;i++)
> -                               buf[i + 26] = file->name[i];
> +                               buf[i + 27] = file->name[i];
>
>                         buf[1] = 0x19 + file->namelen + 2;
>                 }

> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming The Go Parallel Website, sponsored
> by Intel and developed in partnership with Slashdot Media, is your hub for all
> things parallel software development, from weekly thought leadership blogs to
> news, videos, case studies, tutorials and more. Take a look and join the
> conversation now. http://goparallel.sourceforge.net/
> _______________________________________________
> Opensc-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/opensc-devel


--
Frank Morgner

Virtual Smart Card Architecture http://vsmartcard.sourceforge.net
OpenPACE                        http://openpace.sourceforge.net
IFD Handler for libnfc Devices  http://sourceforge.net/projects/ifdnfc

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel



------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: buffer overflow in card-myeid.c

Douglas E Engert


On 3/31/2015 8:59 AM, Peter Popovec wrote:
> Hi,
>
> I do not have a github account and I do not wish to register as a developer on github.

Then at least send your git -diff patch as an attachment.


>
>
> On Tue, Mar 31, 2015 at 2:12 PM, Frank Morgner <[hidden email] <mailto:[hidden email]>> wrote:
>
>     Thanks for the report, the fix looks OK. Could you create a pull request
>     on Github?
>
>     On Monday, March 30 at 09:55AM, Peter Popovec wrote:
>      > Hi,
>      >
>      > I found this in opensc-0.14.0:
>      >
>      > 0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:769:sc_card_ctl: called
>      > 0x7fdf91066700 10:53:44.400 [pkcs15-init] card-myeid.c:1189:myeid_card_ctl:
>      > called
>      > 0x7fdf91066700 10:53:44.400 [pkcs15-init] card-myeid.c:1215:myeid_card_ctl:
>      > returning with: -1408 (Not supported)
>      > 0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:776:sc_card_ctl:
>      > card_ctl(4) not supported
>      > 0x7fdf91066700 10:53:44.400 [pkcs15-init] card.c:414:sc_create_file: called;
>      > type=2, path=3f005015, size=5000
>      > 0x7fdf91066700 10:53:44.400 [pkcs15-init]
>      > card-myeid.c:439:myeid_create_file: called
>      > 0x7fdf91066700 10:53:44.400 [pkcs15-init]
>      > card-myeid.c:311:encode_file_structure: called
>      > *** buffer overflow detected ***: pkcs15-init terminated
>      > ======= Backtrace: =========
>      > /lib/x86_64-linux-gnu/libc.so.6(+0x731ff)[0x7fdf8ff861ff]
>      > /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x37)[0x7fdf900094c7]
>      > /lib/x86_64-linux-gnu/libc.so.6(+0xf46e0)[0x7fdf900076e0]
>      > /usr/lib/x86_64-linux-gnu/libopensc.so.3(+0x98026)[0x7fdf90775026]
>      > /usr/lib/x86_64-linux-gnu/libopensc.so.3(sc_create_file+0xaa)[0x7fdf9070061a]
>      > /usr/lib/x86_64-linux-gnu/libopensc.so.3(sc_pkcs15init_create_file+0x113)[0x7fdf907be063]
>      >
>      > Analysis:
>      >
>      > Function myeid_create_file() allocates buffer of 32 bytes. Then it  calls
>      > encode_file_structure() function. This function allocates buffer of 42
>      > bytes.
>      >
>      > First buffer overflow can be hit if filetype is SC_FILE_TYPE_DF. Function
>      > encode_file_structure() needs 0x19 + file->namelen + 2 bytes in buffer =
>      > max 45 bytes for file->namelen = 16. Buffer is three bytes smaller ..
>      >
>      > Second buffer overflow can be hit at the end of encode_file_structure().
>      > Function  memcpy(out, buf, *outlen) is called at worst case with outlen=45.
>      > Because the space in destination string is only 32 bytes (allocated by
>      > myeid_create_file() function), buffer overflow is generated.
>      >
>      >
>      > Please, use 45 bytes for these buffers:
>      >
>      >
>      > diff --git a/src/libopensc/card-myeid.c b/src/libopensc/card-myeid.c
>      > index 2e5f9c6..babae88 100644
>      > --- a/src/libopensc/card-myeid.c
>      > +++ b/src/libopensc/card-myeid.c
>      > @@ -305,7 +305,7 @@ static int encode_file_structure(sc_card_t *card, const
>      > sc_file_t *file,
>      >                 u8 *out, size_t *outlen)
>      >  {
>      >         const sc_acl_entry_t *read, *update, *delete, *generate;
>      > -       u8 buf[40];
>      > +       u8 buf[45];
>      >         int i;
>      >
>      >         LOG_FUNC_CALLED(card->ctx);
>      > @@ -432,7 +432,7 @@ static int encode_file_structure(sc_card_t *card, const
>      > sc_file_t *file,
>      >  static int myeid_create_file(struct sc_card *card, struct sc_file *file)
>      >  {
>      >         sc_apdu_t apdu;
>      > -       u8 sbuf[32];
>      > +       u8 sbuf[45];
>      >          size_t buflen;
>      >         int r;
>      >
>      >  There is another error encode_file_structure() function:
>      >
>      >        case SC_FILE_TYPE_DF:
>      >                 buf[8] = 0x38;
>      >                 if(file->namelen > 0 && file->namelen <= 16)
>      >                 {
>      >                         buf[25] = 0x84;
>      >                         buf[26] = (u8)file->namelen;
>      >
>      >                         for(i=0;i < file->namelen;i++)
>      >                                 buf[i + 26] = file->name[i];
>      >
>      >                         buf[1] = 0x19 + file->namelen + 2;
>      >                 }
>      >                 break;
>      >
>      >
>      > First, buf[26] is set to actual namelen:
>      >
>      > buf[26] = (u8)file->namelen;
>      >
>      > Then this byte will be overwriten by first character of DF name:
>      >
>      > buf[i + 26] = file->name[i];
>      >
>      > Patch fo tix this issue:
>      >
>      > diff --git a/src/libopensc/card-myeid.c b/src/libopensc/card-myeid.c
>      > index e8c28ec..53f904b 100644
>      > --- a/src/libopensc/card-myeid.c
>      > +++ b/src/libopensc/card-myeid.c
>      > @@ -453,7 +453,7 @@ static int encode_file_structure(sc_card_t *card, const
>      > sc_file_t *file,
>      >                         buf[26] = (u8)file->namelen;
>      >
>      >                         for(i=0;i < file->namelen;i++)
>      > -                               buf[i + 26] = file->name[i];
>      > +                               buf[i + 27] = file->name[i];
>      >
>      >                         buf[1] = 0x19 + file->namelen + 2;
>      >                 }
>
>      > ------------------------------------------------------------------------------
>      > Dive into the World of Parallel Programming The Go Parallel Website, sponsored
>      > by Intel and developed in partnership with Slashdot Media, is your hub for all
>      > things parallel software development, from weekly thought leadership blogs to
>      > news, videos, case studies, tutorials and more. Take a look and join the
>      > conversation now. http://goparallel.sourceforge.net/
>      > _______________________________________________
>      > Opensc-devel mailing list
>      > [hidden email] <mailto:[hidden email]>
>      > https://lists.sourceforge.net/lists/listinfo/opensc-devel
>
>
>     --
>     Frank Morgner
>
>     Virtual Smart Card Architecture http://vsmartcard.sourceforge.net
>     OpenPACE http://openpace.sourceforge.net
>     IFD Handler for libnfc Devices http://sourceforge.net/projects/ifdnfc
>
>     ------------------------------------------------------------------------------
>     Dive into the World of Parallel Programming The Go Parallel Website, sponsored
>     by Intel and developed in partnership with Slashdot Media, is your hub for all
>     things parallel software development, from weekly thought leadership blogs to
>     news, videos, case studies, tutorials and more. Take a look and join the
>     conversation now. http://goparallel.sourceforge.net/
>     _______________________________________________
>     Opensc-devel mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.sourceforge.net/lists/listinfo/opensc-devel
>
>
>
>
> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming The Go Parallel Website, sponsored
> by Intel and developed in partnership with Slashdot Media, is your hub for all
> things parallel software development, from weekly thought leadership blogs to
> news, videos, case studies, tutorials and more. Take a look and join the
> conversation now. http://goparallel.sourceforge.net/
>
>
>
> _______________________________________________
> Opensc-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/opensc-devel
>

--

  Douglas E. Engert  <[hidden email]>


------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: buffer overflow in card-myeid.c

Frank Morgner
I created #421 with the fixes

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel

attachment0 (985 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: buffer overflow in card-myeid.c

Peter Popovec
Hi Frank,

Please, review this patch (src/libopensc/card-myeid.c, function encode_file_structure ) again:

git show 30b24e79c08e6c027ffa3aa8136c10f689ce7a63

-                       buf[1] = 0x19 + file->namelen + 2;
+                       buf[1] = 27 + file->namelen;


This line seems to be  wrong, because APDU for creation of DF with name is still not correct:

Outgoing APDU data [   46 bytes] =====================================
00 E0 00 00 29 62 27 81 02 13 88 82 01 38 83 02 ....)b'......8..
50 15 86 03 11 1F FF 85 02 00 00 8A 01 00 84 0C P...............
A0 00 00 00 63 50 4B 43 53 2D 31 35 00 00       ....cPKCS-15..

At the end of APDU  you can see two bytes 00 00 because length of APDU (from buf[1] ) is wrong.

Correct APDU should look like this:
Outgoing APDU data [   44 bytes] =====================================
00 E0 00 00 27 62 25 81 02 13 88 82 01 38 83 02 ....'b%......8..
50 15 86 03 11 1F FF 85 02 00 00 8A 01 00 84 0C P...............
A0 00 00 00 63 50 4B 43 53 2D 31 35             ....cPKCS-15

I propose to change the calculation of buf[1]:

buf[1] += 2 + file->namelen;

If you agree, please create appropriate pull request.

Thanks
 


On Wed, Apr 1, 2015 at 2:11 AM, Frank Morgner <[hidden email]> wrote:
I created #421 with the fixes

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel



------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: buffer overflow in card-myeid.c

Frank Morgner
On Wednesday, April 08 at 08:31AM, Peter Popovec wrote:

> Hi Frank,
>
> Please, review this patch (src/libopensc/card-myeid.c, function
> encode_file_structure ) again:
>
> git show 30b24e79c08e6c027ffa3aa8136c10f689ce7a63
>
> -                       buf[1] = 0x19 + file->namelen + 2;
> +                       buf[1] = 27 + file->namelen;
>
>
> This line seems to be  wrong, because APDU for creation of DF with name is
> still not correct:
>
> Outgoing APDU data [   46 bytes] =====================================
> 00 E0 00 00 29 62 27 81 02 13 88 82 01 38 83 02 ....)b'......8..
> 50 15 86 03 11 1F FF 85 02 00 00 8A 01 00 84 0C P...............
> A0 00 00 00 63 50 4B 43 53 2D 31 35 00 00       ....cPKCS-15..
>
> At the end of APDU  you can see two bytes 00 00 because length of APDU
> (from buf[1] ) is wrong.
>
> Correct APDU should look like this:
> Outgoing APDU data [   44 bytes] =====================================
> 00 E0 00 00 27 62 25 81 02 13 88 82 01 38 83 02 ....'b%......8..
> 50 15 86 03 11 1F FF 85 02 00 00 8A 01 00 84 0C P...............
> A0 00 00 00 63 50 4B 43 53 2D 31 35             ....cPKCS-15
>
> I propose to change the calculation of buf[1]:
>
> buf[1] += 2 + file->namelen;
>
> If you agree, please create appropriate pull request.
This above change would be wrong (the data would not start with 62 25
but wit 62 0E).

If changed to `buf[1] += 25 + file->namelen;` this should give the correct
ASN.1 encoding if your above APDUs are correct.  However, the code
currently looks OK (i.e. the indices and buf[1] should be correct).

If you want to make a really cool pull request, you should rewrite this
function so that it uses `sc_asn1_encode` which would avoid manually
encoding ASN.1.

Greets, Frank.

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel

attachment0 (985 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: buffer overflow in card-myeid.c

Peter Popovec
Hi

> This above change would be wrong (the data would not start with 62 25 but wit 62 0E).

This is not true,  buf[1] is already set to 0x17. The proposed change only increment this length by two bytes (for tag 0x84, size for tag 0x84) and  real length of filename.

Of course I agree, rewrite it all to use asn1 encoding functions is probably the best solution, but big code changes may introduce new errors.




On Wed, Apr 8, 2015 at 10:42 AM, Frank Morgner <[hidden email]> wrote:
On Wednesday, April 08 at 08:31AM, Peter Popovec wrote:
> Hi Frank,
>
> Please, review this patch (src/libopensc/card-myeid.c, function
> encode_file_structure ) again:
>
> git show 30b24e79c08e6c027ffa3aa8136c10f689ce7a63
>
> -                       buf[1] = 0x19 + file->namelen + 2;
> +                       buf[1] = 27 + file->namelen;
>
>
> This line seems to be  wrong, because APDU for creation of DF with name is
> still not correct:
>
> Outgoing APDU data [   46 bytes] =====================================
> 00 E0 00 00 29 62 27 81 02 13 88 82 01 38 83 02 ....)b'......8..
> 50 15 86 03 11 1F FF 85 02 00 00 8A 01 00 84 0C P...............
> A0 00 00 00 63 50 4B 43 53 2D 31 35 00 00       ....cPKCS-15..
>
> At the end of APDU  you can see two bytes 00 00 because length of APDU
> (from buf[1] ) is wrong.
>
> Correct APDU should look like this:
> Outgoing APDU data [   44 bytes] =====================================
> 00 E0 00 00 27 62 25 81 02 13 88 82 01 38 83 02 ....'b%......8..
> 50 15 86 03 11 1F FF 85 02 00 00 8A 01 00 84 0C P...............
> A0 00 00 00 63 50 4B 43 53 2D 31 35             ....cPKCS-15
>
> I propose to change the calculation of buf[1]:
>
> buf[1] += 2 + file->namelen;
>
> If you agree, please create appropriate pull request.

This above change would be wrong (the data would not start with 62 25
but wit 62 0E).

If changed to `buf[1] += 25 + file->namelen;` this should give the correct
ASN.1 encoding if your above APDUs are correct.  However, the code
currently looks OK (i.e. the indices and buf[1] should be correct).

If you want to make a really cool pull request, you should rewrite this
function so that it uses `sc_asn1_encode` which would avoid manually
encoding ASN.1.

Greets, Frank.

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel



------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: buffer overflow in card-myeid.c

Frank Morgner
On Wednesday, April 08 at 12:26PM, Peter Popovec wrote:
> Hi
>
> > This above change would be wrong (the data would not start with 62 25 but
> wit 62 0E).
>
> This is not true,  buf[1] is already set to 0x17. The proposed change only
> increment this length by two bytes (for tag 0x84, size for tag 0x84) and
> real length of filename.

Sorry, you're right. I misread `+=` as `=`

> Of course I agree, rewrite it all to use asn1 encoding functions is
> probably the best solution, but big code changes may introduce new errors.

The current situation does not seem to be perfect either...


--
Frank Morgner

Virtual Smart Card Architecture http://vsmartcard.sourceforge.net
OpenPACE                        http://openpace.sourceforge.net
IFD Handler for libnfc Devices  http://sourceforge.net/projects/ifdnfc

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel

attachment0 (985 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: buffer overflow in card-myeid.c

Peter Popovec
Hi,

I agree, function encode_file_structure() is suitable to be completely rewriten ( do not use cycle for filename copy, memcpy is better solution,  do not allocate new buffer on stack, use  already allocated buffer from myeid_create_file(). Despite of all this, it is usable code.

Peter





On Thu, Apr 9, 2015 at 7:30 AM, Frank Morgner <[hidden email]> wrote:
On Wednesday, April 08 at 12:26PM, Peter Popovec wrote:
> Hi
>
> > This above change would be wrong (the data would not start with 62 25 but
> wit 62 0E).
>
> This is not true,  buf[1] is already set to 0x17. The proposed change only
> increment this length by two bytes (for tag 0x84, size for tag 0x84) and
> real length of filename.

Sorry, you're right. I misread `+=` as `=`

> Of course I agree, rewrite it all to use asn1 encoding functions is
> probably the best solution, but big code changes may introduce new errors.

The current situation does not seem to be perfect either...


--
Frank Morgner

Virtual Smart Card Architecture http://vsmartcard.sourceforge.net
OpenPACE                        http://openpace.sourceforge.net
IFD Handler for libnfc Devices  http://sourceforge.net/projects/ifdnfc

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel



------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel