Skip to content

dist/tools/tunslip: fix some minor code quality issues#21977

Merged
maribu merged 1 commit intoRIOT-OS:masterfrom
maribu:dist/tools/tunslip
Jan 12, 2026
Merged

dist/tools/tunslip: fix some minor code quality issues#21977
maribu merged 1 commit intoRIOT-OS:masterfrom
maribu:dist/tools/tunslip

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Jan 12, 2026

Contribution description

  • drop an unused include
  • drop a duplicate forward declaration
  • fix some minor code style issues
  • add check for being started with a command line that contains a /dev/tty... device paths that is longer than 1024 chars.
    • This improves UX slightly by turning a segfault on invalid input into a proper error message
    • The person starting the tool is trusted to start commands. It might be possible to get code execution by pwning the tool via the buffer overflow, but one would need to have code execution in order to start the tool with a malicious command line arg...

Testing procedure

  • There should be no observable change in behavior when the tool is run correctly
  • When the tool is run with some /dev/<very-very-very-long-path> as argument, it should now fail more gracefully than a segfault
  • Compiling tapslip6 should no longer emit warnings (but does so in master)

In master

./tapslip6 -s /dev/$(python3 -c 'print("A"*4096)') 192.168.0.1 255.255.255.0
*** stack smashing detected ***: terminated
[1]    48295 IOT instruction (core dumped)  ./tapslip6 -s /dev/$(python3 -c 'print("A"*4096)') 192.168.0.1 255.255.255.0

This PR

./tapslip6 -s /dev/$(python3 -c 'print("A"*4096)') 192.168.0.1 255.255.255.0
tapslip6: can't open siodev ``/dev/AAA[...]AAA'': Invalid argument

(Note: The /dev/AAA[...]AAA truncation is done manually, the actual output contained the full 4095 As.)

Issues/PRs references

https://seclists.org/fulldisclosure/2026/Jan/15

- drop an unused include
- drop a duplicate forward declaration
- fix some minor code style issues
- add check for being started with a command line that contains a
  `/dev/tty...` device paths that is longer than 1024 chars.
    - This improves UX slightly by turning a segfault on invalid input
      into a proper error message
    - The person starting the tool is trusted to start commands. It might
      be possible to get code execution by pwning the tool via the
      buffer overflow, but one would need to have code execution in
      order to start the tool with a malicious command line arg...
@github-actions github-actions bot added the Area: tools Area: Supplementary tools label Jan 12, 2026
@maribu maribu added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 12, 2026
@riot-ci
Copy link
Copy Markdown

riot-ci commented Jan 12, 2026

Murdock results

✔️ PASSED

30791db dist/tools/tunslip: fix some minor code quality issues

Success Failures Total Runtime
1 0 1 01m:13s

Artifacts

in_addr_t netaddr;
in_addr_t circuit_addr;

int ssystem(const char *fmt, ...) __attribute__((__format__(__printf__, 1, 2)));
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here is the first declaration of ssystem(). The duplicate below is removed. This is sadly not part of what shows up in the GitHub diff.

@maribu maribu added this pull request to the merge queue Jan 12, 2026
Merged via the queue into RIOT-OS:master with commit 6e06b38 Jan 12, 2026
29 checks passed
@leandrolanzieri leandrolanzieri added this to the Release 2026.01 milestone Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants