On Sat, Sep 27, 2025 at 08:26:17PM +0100, Harald van Dijk wrote:
> On 27/09/2025 19:59, Osama Abdelkader wrote:
> > Replace all sprintf calls with snprintf to prevent potential buffer
> > overflows
> > when formatting /proc paths with variable PID values.
>
> In the context of your first change, it shows filename is defined as
>
> char filename[sizeof("/proc/%u/smaps") + sizeof(int)*3];
>
> to ensure that the buffer is always large enough. The same looks true for
> the others as well.
>
> If you want to change busybox to account for the possibility that the buffer
> size calculation is wrong anywhere, then please keep in mind that operating
> on a truncated file name, as your patch would make it do, would be doing the
> wrong thing.
>
> Cheers,
> Harald van Dijk
Thank you for the feedback. You're absolutely correct.
After analyzing the buffer size calculations:
- sizeof("/proc/%u/smaps") = 15
- sizeof(int)*3 = 12
- Total buffer = 27 bytes
- Max PID string length = 10 bytes
- Result: "/proc/2147483647/smaps" = 22 bytes
The buffers are indeed correctly sized to handle maximum PID values.
My patch would have caused truncation, leading to incorrect behavior.
I'll look for actual memory safety issues where buffers are genuinely
undersized or bounds checking is missing.
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
