Re: tgtd buffer overflow and command injection vulnerabilities

While looking into making a patch for this issue I have found another
buffer overflow in iscsi/target.c for the same callback feature in the
function target_redirected:

char dst[INET6_ADDRSTRLEN], in_buf[1024];
...

p = in_buf;
		p += sprintf(p, "%s ", target->redirect_info.callback);
		p += sprintf(p, "%s ", tgt_targetname(target->tid));
...
sprintf(p, "%s", dst);


Where target->redirect_info.callback is char buffer set by user input and
can easily be over 1024 characters. Having gone over these functions I'm
not exactly clear what it's purpose is, so perhaps someone on the tgt side
would be better suited to fix these issues. I would recommend not using
sprintf (or other such unsafe functions) throughout the tgt project and at
least using snprintf instead.

Thanks,

Jason Hullinger



On 6/16/14, 1:06 PM, "Hullinger, Jason (Cloud Services)"
<[email protected]> wrote:

>Hi,
>
>Thanks for the clarification, and I see you are using a domain socket at
>/var/run/tgtd.ipc_abstract_namespace.X Since the overflow occurs in a
>function that is expected to do arbitrary commands it's sort of redundant
>as a security issue. It is a bug though and will cause the process to
>break so it should still be fixed.
>
>Thanks,
>
>Jason Hullinger
>
>On 6/14/14, 6:29 AM, "FUJITA Tomonori" <[email protected]>
>wrote:
>
>>Sorry about the delay,
>>
>>On Tue, 10 Jun 2014 19:17:35 +0000
>>"Hullinger, Jason (Cloud Services)" <[email protected]> wrote:
>>
>>> The function call_program in the tgtd daemon includes a callback
>>>function
>>> that will run arbitrary commands. Additionally, it does not check that
>>>the
>>
>>Yeah, the feature is intentional:
>>
>>http://www.spinics.net/lists/linux-stgt/msg02065.html
>>
>>No security about tgtadm. A user who can use tgtadm has the root
>>permission. He can do whatever he want to on the machine. He doesn't
>>need to use a security hole in tgtd and tgtadm.
>>
>>Of course, we care about security about iscsi and isns ports.