Skip to content

Conversation

@linear0211
Copy link
Contributor

The --addr-pool option previously used atoi to parse the mask value.
If non-numeric characters were included, atoi returned 0, causing the pool to unintentionally accept all IP addresses.

This change ensures the mask accepts only numeric values, preventing unexpected behavior.

@lum1n0us lum1n0us added the bug-fix Determine if this PR addresses a bug. It will be used by scripts to classify PRs. label Sep 11, 2025
@linear0211 linear0211 requested a review from lum1n0us September 27, 2025 07:19
lum1n0us
lum1n0us previously approved these changes Oct 12, 2025
Copy link
Collaborator

@lum1n0us lum1n0us left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. With minor comments

linear0211 and others added 2 commits October 13, 2025 16:02
#else
address = strtok_r(cp, "/", &nextptr);
mask = strtok_r(NULL, "/", &nextptr);
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can have bh_strtok_r function for this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. However, I think we should use a new PR to implement that and merge it first. Does that sound good to you?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. Should I open a new PR for the implementation?

#else
address = strtok_r(cp, "/", &nextptr);
mask = strtok_r(NULL, "/", &nextptr);
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. However, I think we should use a new PR to implement that and merge it first. Does that sound good to you?

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

Labels

bug-fix Determine if this PR addresses a bug. It will be used by scripts to classify PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants