On 12/11/2012 3:27 PM, Frank Morgner wrote: > On Tuesday, December 11 at 09:59AM, Douglas E. Engert wrote: >> >> >> >> On 12/7/2012 5:15 PM, Frank Morgner wrote: >>> Hi! >>> >>> Currently, sc_check_apdu checks the length of an R-APDU buffer using >>> SC_MAX_APDU_BUFFER_SIZE, which defines the maximum length for a C-APDU. >>> https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/apdu.c#L415 >>> https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/apdu.c#L392 >> >> Yes this looks like a bug as SC_MAX_APDU_BUFFER_SIZE is for max size of ADPU >> that can be sent, not size of receive buffer: >> #define SC_MAX_APDU_BUFFER_SIZE 261 /* takes account of: CLA INS P1 >> P2 Lc [255 byte of data] Le */ >> >> >>> >>> A quick fix would be to use 0xff+1/0xffff+1 instead. However, in >>> multiple files I have seen this wrong usage of SC_MAX_APDU_BUFFER_SIZE >>> (eg, see `grep rbuf *.c | grep SC_MAX_APDU_BUFFER_SIZE`). Unfortunately >>> I dont have time to check libopensc in depth, so I leave this up to the >>> community. >>> >> >> Do you mean something like this: >> >> --- ,apdu.c Tue Dec 4 08:43:40 2012 >> +++ apdu.c Tue Dec 11 09:50:50 2012 >> @@ -389,7 +389,7 @@ >> if (apdu->resplen == 0 || apdu->resp == NULL) >> goto error; >> /* return buffer to small */ >> - if ((apdu->le == 0 && apdu->resplen < >> SC_MAX_APDU_BUFFER_SIZE-2) >> + if ((apdu->le == 0 && apdu->resplen < ((apdu->cse & >> SC_APDU_EXT) ? 65536 : 256)) >> || (apdu->resplen < apdu->le)) >> goto error; >> break; >> @@ -412,7 +412,7 @@ >> if (apdu->resplen == 0 || apdu->resp == NULL) >> goto error; >> /* return buffer to small */ >> - if ((apdu->le == 0 && apdu->resplen < >> SC_MAX_APDU_BUFFER_SIZE-2) >> + if ((apdu->le == 0 && apdu->resplen < ((apdu->cse & SC_APDU_EXT) ? >> 65536 : 256) >> || (apdu->resplen < apdu->le)) >> goto error; >> /* inconsistent datalen */ > > Yes, but I would use a define instead of hard coded values.
The 65536 and 256 are also used in other places in the module, so I used the numbers. That not to say that defines could not be used. Did you just notice the code looked wrong or do you have a problem caused by the original code? Could you test a change? > Please have > in mind that the rest of the source code should be checked, too. The > following grep shows 65 hits which should be changed to use the new > define: > > grep -R SC_MAX * |egrep '(rbuf|recvbuf)' Fortunately these buffers are 261 bytes long, as the define was meant to define the max size that could be sent, and this is larger then the size that can be received. So although the 65 places could be changed, the use of the buffers in every instance would need to be reviewed. There may be other locations that use the SC_MAX_APDU_BUFFER_SIZE that don't use rbuf or recvbuf. Can you provide a change? > > > > _______________________________________________ > opensc-devel mailing list > opensc-devel@lists.opensc-project.org > http://www.opensc-project.org/mailman/listinfo/opensc-devel > -- Douglas E. Engert <deeng...@anl.gov> Argonne National Laboratory 9700 South Cass Avenue Argonne, Illinois 60439 (630) 252-5444 _______________________________________________ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
