[tac_plus] Patch for a crash when using long commands

[heasley]

Mon, Jan 20, 2014 at 04:38:32PM +0100, Marc Dequenes:
> Coin,
> 
> One of our clients had a bug wich seemed to happen only with long commands like:
>   path 1 e1 15  tu12-au4 1 3 framing crc4 mapping-mode bit-async
> timeslots 1-9 10-18 19-27
> (on CISCO routers, probably CRS)
> 
> The child process crashed with:
> *** glibc detected *** /usr/bin/tac_plus: free(): invalid next size
> (fast): 0x000000001eea4d60 ***
> ======= Backtrace: =========
> /lib64/libc.so.6[0x37aaa7230f]
> /lib64/libc.so.6(cfree+0x4b)[0x37aaa7276b]
> /usr/bin/tac_plus[0x40a205]
> /usr/bin/tac_plus[0x403af6]
> /usr/bin/tac_plus[0x40f2ea]
> /usr/bin/tac_plus[0x40fc24]
> /lib64/libc.so.6(__libc_start_main+0xf4)[0x37aaa1d994]
> /usr/bin/tac_plus[0x402939]
> 
> I found an off-by-one mistake in the command buffer allocation (room
> for \0 was forgotten) and made a small patch to fix it. We are unable
> to reproduce since then.

I suspect you're just getting lucky.  looking at that code, its adding 1
byte for each argument.  the the space calculation appears to be excessive.

the glib error would imply that the something overstepped bounds and mashed
malloc bookkeeping data.

so, can you tell me more about the device configuration, the tacacs daemon
configuration and provide a backtrace from the core file?

This would check that theory:

Index: do_author.c
===================================================================
--- do_author.c (revision 3626)
+++ do_author.c (working copy)
@@ -395,9 +395,12 @@
            free(buf);
            return(NULL);
        }
-       strcat(buf, v);
-       if (i < (data->num_in_args - 1))
-           strcat(buf, " ");
+       strncat(buf, v, len - 1);
+       len -= strlen(v);
+       if (i < (data->num_in_args - 1)) {
+           strncat(buf, " ", len - 1);
+           len -= 1;
+       }
     }
     return(buf);
 }

> Regards.
> 
> -- 
> Marc Dequènes
> Consultant Devoteam Uperto/IdeOS
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: tacacs+_buffer_alloc_offbyone.patch
> Type: text/x-patch
> Size: 408 bytes
> Desc: not available
> URL: <http://www.shrubbery.net/pipermail/tac_plus/attachments/20140120/4f2a1871/attachment.bin>
> _______________________________________________
> tac_plus mailing list
> tac_plus at shrubbery.net
> http://www.shrubbery.net/mailman/listinfo/tac_plus