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

[ros2 turtlesim] turtle.cpp needs to validate float32/float64 input variables #129

Open
squizz617 opened this issue Jul 3, 2021 · 1 comment
Labels
bug help wanted Extra attention is needed

Comments

@squizz617
Copy link
Contributor

There are many places in the code that assume control inputs (i.e., x, y, and theta) are valid floating point numbers. As a result, when one of nan, -nan, inf or -inf is given, the turtle's position becomes infeasible (e.g., x being nan), sometimes even become rendering it uncontrollable.

  • Case 1: teleport_absolute service
    • In Turtle::update(), the requested x is directly used to set position (pos_.setX(req.pos.x());).
    • When nan or -nan is provided, turtle's x position becomes nan.
    • Requests with theta set to nan or inf make x, y, and theta of the turtle's pose nan, removing the turtle from the frame.
  • Case 2: teleport_relative service
    • Similarly, if linear and/or angular of teleport_relative request is set to nan or inf, turtle's position becomes infeasible.
  • Case 3: rotate_absolute action
    • Sending goal with theta set to nan or inf can have the turtle rotate indefinitely, as remaining becomes either nan or -nan.
    • This forces ang_vel_ to become 1.0 after executing the following statement: ang_vel_ = remaining < 0.0 ? -1.0 : 1.0;, as nan < 0.0 always evaluates to False.
  • Case 4: cmd_vel topic
    • linear.x, linear.y, and angular.z are not sanitized before being used. Similar to the previous cases, x, y and/or theta of turtle's pose are easily set to nan.

I suggest adding checks before using these variables, e.g., std::isnan and std::isinf, to prevent unexpected input values from polluting the turtle's state.

@audrow audrow added bug help wanted Extra attention is needed labels Dec 20, 2021
@audrow
Copy link
Contributor

audrow commented Dec 20, 2021

Hi @squizz617, thanks for making this issue. Feel free to make a PR with some or all of these fixes. If you do, you can @ me for a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants