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

Fix: Prevent segmentation fault when setting mana_credout or sycophant_dir #42

Closed
wants to merge 2 commits into from

Conversation

alxbl
Copy link

@alxbl alxbl commented Nov 15, 2019

I ran into this issue on a recent engagement. I recall it working before so it must have been introduced at some point.

The issue is that the mana_credout and sycophant_dir config entries are not initialized by hostapd_config_defaults and are freed without check when the config file is parsed.

The fix is simply to not free the strings, since these configurations cannot change at runtime anyway (As far as I can tell from a quick glance at the codebase, at least) .

I'm not sure if mana is still being maintained, but I thought I might as well share the changes with the community.

Cheers,
Alex

@alxbl
Copy link
Author

alxbl commented Nov 15, 2019

After a little bit of digging, the issue was introduced in 8b529e7#diff-0034d76c981823550f0ad389932e838dR2188 which was prompted by LGTM suggestions. I think the os_free is a false positive here.

Alternatively, both settings could be initialized to reasonable defaults (NULL for mana_credout and /tmp/ for sycophant_dir and the os_free could be left in. The last word is yours.

Cheers,

@singe
Copy link
Contributor

singe commented Nov 15, 2019 via email

@singe
Copy link
Contributor

singe commented Jan 21, 2020

I reverted the lgtm fixes I put in place that caused this.

@singe singe closed this Jan 21, 2020
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