Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

dhcp: get_option_uint32/16 only accept options with correct len #357

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

taoyl-g
Copy link
Contributor

@taoyl-g taoyl-g commented Aug 28, 2024

RFC8925 mentions "The client MUST ignore the IPv6-Only Preferred option if the length field value is not 4."

@taoyl-g
Copy link
Contributor Author

taoyl-g commented Aug 28, 2024

I'm not modifying get_option_uint8 now as it's also used to read variant length options. Please suggest if you have other preferred way to fix this (such as adding another helper that does not verify the length)

@rsmarples
Copy link
Member

I'm not modifying get_option_uint8 now as it's also used to read variant length options. Please suggest if you have other preferred way to fix this (such as adding another helper that does not verify the length)

That would be the correct way I think.
We have two calls that use the uint8 function just to check the existence of an option so we could have a new function that does just that.

RFC8925 mentions "The client MUST ignore the IPv6-Only Preferred option
if the length field value is not 4."
@taoyl-g
Copy link
Contributor Author

taoyl-g commented Sep 4, 2024

Enforcing len==1 in get_option_uint8 as well, and using get_option instead in the mentioned occurrences.

src/dhcp.c Show resolved Hide resolved
src/dhcp.c Show resolved Hide resolved
@rsmarples
Copy link
Member

Heh, I should look at the larger picture some more. Thanks for your work and replies.

@rsmarples rsmarples merged commit 72a2628 into NetworkConfiguration:master Sep 5, 2024
2 of 5 checks passed
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.

2 participants