Fixes two bugs in the single shell riscv payloads#21519
Conversation
First: passes the path in rather than null (busybox crashes) Second: fixes a sign extension bug in the port loading
There was a problem hiding this comment.
Pull request overview
This PR fixes two correctness issues in the Linux RISC-V reverse TCP single-command shell payloads: (1) ensure execve receives a non-NULL argv (BusyBox compatibility), and (2) fix load_const_into_reg64 so the low 32-bit half can’t sign-extend and corrupt the high 32-bit half during constant construction.
Changes:
- Update RISC-V 32/64 reverse TCP shellcode to call
execve("/bin/sh", ["/bin/sh", NULL], NULL)and adjust stack/CachedSize accordingly. - Fix
load_const_into_reg64in the riscv64 reverse TCP payload by zero-extending the low half viaslli+srlibefore theor. - Add new RSpec coverage validating CachedSize, non-NULL
argvsetup, and the sign-extension regression case.
Impact Analysis:
- Blast radius: medium — affects all users generating
linux/riscv32le/shell_reverse_tcpandlinux/riscv64le/shell_reverse_tcppayloads; also affects any encoders/stagers that wrap these payload bytes. - Data and contract effects: no persistent data/schema changes; payload byte layout and total size (
CachedSize) changes for both architectures (must remain consistent with framework expectations). - Rollback and test focus: low rollback risk (isolated shellcode changes), but validation should focus on: payload generation size checks, reverse TCP connectivity, and
execvebehavior on BusyBox-based targets (argv handling) plus the LPORT=4481 regression for riscv64.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
modules/payloads/singles/linux/riscv64le/shell_reverse_tcp.rb |
Fix 64-bit const construction sign-extension bug; pass non-NULL argv to execve; increase stack usage and CachedSize. |
modules/payloads/singles/linux/riscv32le/shell_reverse_tcp.rb |
Pass non-NULL argv to execve; adjust CachedSize accordingly. |
spec/modules/payloads/singles/linux/riscv64le/shell_reverse_tcp_spec.rb |
New tests for CachedSize, BusyBox argv setup, and riscv64 sign-extension regression coverage. |
spec/modules/payloads/singles/linux/riscv32le/shell_reverse_tcp_spec.rb |
New tests for CachedSize and BusyBox argv setup coverage. |
An issue related to BusyBox on 64-bit was documented as WontFix in the original PR (#19518):
|
This fixes two bugs I found in the single command shell payloads for riscv while testing #21235:
execve. For some reason, on my version of Busybox for riscv32, passingNULLforpathresulted in a crash. I did not see that behavior on the riscv64 host, and I see that we passNULLin on other IoT architectures, so I'm not sure why it seems to be an issue on this? I imagine it is that we have not hit a busybox target? RISCV32 is sort of an academic arch when it comes to running a Linux kernel, so 🤷I added the change to riscv64, but I am open to removing it there, though busybox is common enough on small stuff.
See https://stackoverflow.com/questions/19043700/busybox-in-embedded-linux-shows-applet-not-found
This change required an increase to the stack size.
load_const_into_reg64method. When using a value of 4481 for the LPORT, theencode_orsign-extended when it should not have.