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

airodump-ng: illegally modifying const variable causing crash #2184

Closed
dracode opened this issue Sep 23, 2020 · 3 comments
Closed

airodump-ng: illegally modifying const variable causing crash #2184

dracode opened this issue Sep 23, 2020 · 3 comments

Comments

@dracode
Copy link

dracode commented Sep 23, 2020

  • Defect - Crash

System information

  • OS: Android

  • CPU: Cortex-A53

  • Version: 1.6

Defect

When no channel list is user-provided, channel list defaults to the list contained in the bg_chans variable:
lopt.channels = (int *) bg_chans;

bg_chans is defined as:
static const int bg_chans[] = {1, 7, 13, 2, 8, 3, 14, 9, 4, 10, 5, 11, 6, 12, 0};

if one of the channels is found to be invalid, lopt.channels is modified:
lopt.channels[ch_idx] = -1;

thus attempting to modify a const, bg_chans, causing a crash.

How to reproduce the issue

Run airodump-ng on a device that can't tune to channel 13.

@dracode
Copy link
Author

dracode commented Sep 23, 2020

The simplest fix for this would be to make abg_chans, bg_chans, and a_chans not consts, as they are all potential sources for lopt.channels.

An alternate fix could be to malloc() enough memory to hold the largest set (abg_chans) and then memcpy the contents into that in the appropriate places, allowing the basic sets to remain consts.

I can generate a PR for either option, depending on the consensus.

@dracode
Copy link
Author

dracode commented Sep 28, 2020

I should add, this bug only really manifests itself when compiled without libnl support. When changing channels via ioctl(), airodump gets feedback on the success/failure of the channel set message.

When using libnl, airodump appears to only validate channel set messages when in single-channel mode. One mildly annoying side effect of this (that should probably be its own issue) is that, in libnl mode, airodump will never recognize that the driver doesn't support a particular channel and will continue to try to access it forever.

@jbenden jbenden closed this as completed in 774d4e0 Nov 8, 2020
@jbenden
Copy link
Collaborator

jbenden commented Nov 8, 2020

Thanks for the detailed report!

Best regards,
-Joe

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

No branches or pull requests

2 participants