Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Eap: Fix check for invaild message hadling #749

Merged
merged 3 commits into from
Feb 2, 2025
Merged

Conversation

Jojeker
Copy link
Contributor

@Jojeker Jojeker commented Jan 23, 2025

Hello :)

We want to submit some patches that target the EAP message parsing and the handling for authentication. There are three issues that we identified and fixed in this series:

  1. An invalid message type (which leads to anullptr is not handled correctly for the authentication functions (e.g. NasMm::receiveAuthenticationRequestEap), since the IE might evalute to nullptr although the message is not valid. This leads to segmentation faults during execution.

  2. An EAP message with invalid length fields (a length < 2 for attributes) leads to an unbounded read in readOctetString, since the last bound is smaller then the first bound.

  3. They enum type bounds for EAP structs are not checked upon parsing, although they provide an early information on processing. This is relevant for parsing the concrete EAPAttributes.

We are happy to provide concrete pcap traces if necessary.

Cheers

Signed-off-by: Eduard Vlad <eduard.vlad@rwth-aachen.de>
Otherwise, the octet reading crashes with inverted bounds

Signed-off-by: Eduard Vlad <eduard.vlad@rwth-aachen.de>
Allowing to provide concrete bounds with typed enums.

Signed-off-by: Eduard Vlad <eduard.vlad@rwth-aachen.de>
@Jojeker Jojeker changed the title Eap: Fix invalid handling for invaild message hadling Eap: Fix check for invaild message hadling Jan 23, 2025
@aligungr
Copy link
Owner

aligungr commented Feb 2, 2025

Thanks

@aligungr aligungr merged commit afede5d into aligungr:master Feb 2, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants