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
Contributor

@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

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

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
Contributor

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

either ways are fine for me

Copy link
Contributor

@lum1n0us lum1n0us Dec 21, 2025

Choose a reason for hiding this comment

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

File #4766 to track

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

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?

@lum1n0us lum1n0us merged commit 0746640 into bytecodealliance:main Dec 21, 2025
463 checks passed
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