From mboxrd@z Thu Jan 1 00:00:00 1970 From: mathstuf at gmail.com (Ben Boeckel) Date: Sun, 28 Oct 2012 01:29:17 +0000 (UTC) Subject: [PATCHv2 2/3] Add ability to authorize viewing a repository References: <1350378927-10834-1-git-send-email-valentin.haenel@gmx.de> <1350894558-24840-2-git-send-email-valentin.haenel@gmx.de> Message-ID: On Sun, Oct 28, 2012 at 01:16:36 GMT, Jason A. Donenfeld wrote: > On Sat, Oct 27, 2012 at 7:00 PM, Ben Boeckel wrote: >> Single quote the arguments to the executable. This is ripe for code >> execution (remote_user is under attacker's control). > > Was going to mention this myself, but you beat me too it. Dead on. > Correctamundo. > > Please double double tripe triple check your code before submitting things. Working on the uzbl shell scripts (where we require POSIX compatibility as well) has made these problems stick out like sore thumbs :) . Alas, secure shell coding isn't the default mode for many programmers :( . I'll look at the other shell code in cgit this weekend. > While we're on the topic, is "system" the best way to be calling this? > Since an auth helper gets called for each and every request, it seems > like it'd be cleanest if we could just fork/exec/wait ourselves, > passing the options in to execv. This way we don't have to fire up a > shell interpreter each time. Well, that would require that we either shell-split the option (messy and error prone in and of itself) or effectively require trampoline scripts for everything due to enforced arguments. Though...something taking these arguments naturally is unlikely. +1 for fork/exec here. --Ben