Skip to content

Conversation

@lunt7
Copy link

@lunt7 lunt7 commented Mar 23, 2020

getAssertionState.creds[ALLOW_LIST_MAX_SIZE] has space for
ALLOW_LIST_MAX_SIZE credentials but we are only filling it up to
ALLOW_LIST_MAX_SIZE - 1

@conorpp
Copy link
Member

conorpp commented Mar 27, 2020

Since C arrays start at 0, I believe GA->creds[ALLOW_LIST_MAX_SIZE - 1]; is the last element?

@jolo1581
Copy link
Contributor

jolo1581 commented Mar 29, 2020

@conorpp you are right n-1 ist the last element of the array. But in this case this is an error exception if your counter is outside array size. So n-1 is valid and n not.

Also it is not a good style asking for equal. Better it would be:

if (count > ALLOW_LIST_MAX_SIZE-1)

or

if (count >= ALLOW_LIST_MAX_SIZE)

So If @lunt7 would correct this to one of the example above and it would be right.

Greetz Jan

count is an index into the creds array in getAssertionState, and it is checked against the array bounds before data are copied.
The array contains space for ALLOW_LIST_MAX_SIZE entries but we are only filling it up to ALLOW_LIST_MAX_SIZE - 1 since the loop terminates early.
@lunt7 lunt7 force-pushed the fix_filter_credentials branch from 8913f17 to 0955619 Compare March 30, 2020 18:51
@lunt7
Copy link
Author

lunt7 commented Mar 30, 2020

There is a similar condition checking in save_credential_list(), so I decided to go with the first one to be consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants