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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰[BUG] - registry_get_plugged_type uses incorrect bounds-check for port index. #623

Open
Tropix126 opened this issue Jan 14, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@Tropix126
Copy link

Tropix126 commented Jan 14, 2024

Describe the bug
This is a fun one.

  • pros::c::registry_get_plugged_type supposedly takes a zero-indexed smart port index and returns the assocaited v5_device_e_t.
  • The function supposedly returns ENXIO (errno 6) if the provided port is out of range (docs claim the range is 0-20, which makes sense, given there are 21 smart ports and this argument is zero-indexed).
    • Despite this, the docs never describe what's actually returned when errno is set.
    • From looking at source code, the function actually returns -1 to intentionally overflow the return value (v5_device_e_t -- which the compiler assumes is a unsigned 8-bit integer) to 255 (which is the value of E_DEVICE_UNDEFINED).

Okay, so when you pass an out of bounds port, registry_get_plugged_type will return E_DEVICE_UNDEFINED (255 overflowed from -1) and set errno to ENXIO, right? Nope! Specifically when requesting port 21's plugged type (a supposedly out-of-bounds port), pros::c::registry_get_plugged_type will return 12, which is E_DEVICE_ADI. Huh?

  • It seems like registry_get_plugged_type bound-checks the port number against VALIDATE_PORT_NO, which uses a max input value of 22. This would work fine if the argument wasn't a zero-indexed port, but in this case it allows us to index into registry_types one port index above the intended maximum. For some reason, this happens to be 12.
    • I can only speculate at the actual reason, but I'm assuming this is due to the registry being initialized using V5_MAX_DEVICE_PORTS which has some internal V5 device access stuff after the regular smart port indexes, and vexDeviceGetStatus happens to store INTERNAL_ADI_PORT at index 21 (which is the brain's "internal ADI expander").

To Reproduce

pros::c::registry_get_plugged_type(21) == 12; // true!

Expected behavior

pros::c::registry_get_plugged_type(21) == E_DEVICE_UNDEFINED;
errno == ENXIO;

Screenshots
N/A

Desktop (please complete the following information):

  • OS: Windows 11 23H2 (26020.1000)

Additional context
Tested on PROS 3.8.0

@Tropix126 Tropix126 added the bug Something isn't working label Jan 14, 2024
@Tropix126
Copy link
Author

This bug very likely applies to other device registry-related functions in apix.h, as well as some internal ones probably. To list a few:

  • registry_get_bound_type
  • registry_unbind_port (wonder what happens when I try to unbind the internal ADI slots)
  • registry_bind_port

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant