Skip to content

Commit

Permalink
Avoid buffer overrun of we got too short response
Browse files Browse the repository at this point in the history
This issue was pointed out by coverity that the memmove could be called
with underflow value, if we get small answer from CCID_Receive(). My
proposal would be to add a sanity check to prevent that, but I am not
that much familiar with all the code so I am open to other proposals or
suggestions.

"Error: OVERRUN (CWE-119):
ccid-1.5.5/src/commands.c:594: write_constant: Write the value 2 into ""*RxLength"".
ccid-1.5.5/src/commands.c:603: overrun-buffer-arg: Calling ""memmove"" with ""RxBuffer"" and ""*RxLength - 4U"" is suspicious because of the very large index, 4294967294. The index may be due to a negative parameter being interpreted as unsigned. [Note: The source code implementation of the function has been overridden by a builtin model.]

 601|
 602|   			/* get only the T=1 data */
 603|-> 			memmove(RxBuffer, RxBuffer+3, *RxLength -4);
 604|   			*RxLength -= 4;	/* remove NAD, PCB, LEN and CRC */
 605|   		}"
  • Loading branch information
Jakuje authored and LudovicRousseau committed May 21, 2024
1 parent 50fced3 commit 346c3aa
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,10 @@ RESPONSECODE SecurePINVerify(unsigned int reader_index,
#define T1_S_TYPE(pcb) ((pcb) & 0x0F)
#define T1_S_WTX 0x03

/* this should not happen. It will make coverity happy */
if (*RxLength < 4)
return IFD_COMMUNICATION_ERROR;

/* WTX S-block */
if ((T1_S_BLOCK | T1_S_WTX) == RxBuffer[PCB])
{
Expand Down

0 comments on commit 346c3aa

Please sign in to comment.