On Mon, Nov 29, 2021 at 12:20:15AM +0000, M Champion <debacle...@rs432.net> wrote:
> Hi raf, > > Replies in-line ; > > > > > Dear Postfix users, > > > > > > I'm really grateful to Wietse who thankfully raised concerns regarding > > > Perl > > > querying the verified senders database using postmap via the shell as > > > there > > > was a real chance the sender (easily faked) could carry out evil. My use > > > of > > > 'backticks' was a very bad idea as it turns out. I managed to escape the > > > script myself so very easy to professional exploiters to do so. Lesson > > > learned the Wietse way, not the way that ends up with flames and spilt > > > tears! > > > > > > I think(?) that I have found the solution courtesy of > > > https://www.w3.org/Security/faq/.back/wwwsf5.html and perldocs.org (open). > > > I modified one of the W3C examples and shoved it into my function : > > > > > > sub postmap > > > { > > > my $sender=shift; > > > $senderchk=$sender; > > > $senderchk =~ s/[\$#~!&*{}()\[\];,:?^ `\\\/]+//g; # check for > > > undesirable chars even if legal > > > if($sender ne $senderchk) { return "9:0:0:Database not probed. Suspect > > > characters detected in sender address.)" } > > > my $sf ="lmdb:/var/lib/postfix/verified_senders_2021"; > > > $vsresult=""; > > > $perlfork = open(POSTMAP,"-|"); die "Couldn't open perl fork" unless > > > defined($perlfork); > > > exec "/usr/sbin/postmap", "-fq", "$sender", "$sf", or die "Couldn't > > > execute postmap" if $perlfork == 0; > > > while (<POSTMAP>) { $vsresult= "$_"; } > > > close POSTMAP; > > > return "$vsresult"; > > > } > > > > > > > > > I'm hoping this will be that last on this subject but I'll put it out to > > > you > > > who have a superior knowledge of security. Is this method now safe? > > > Please, > > > if anyone can see any security issues, do let me know. Apologies to the > > > Perl > > > purists. I'm pretty sure I could have done better there, but it does work > > > .My main concern is if its safe. > > > > > > Many thanks to you all, > > > > > > > > > Best wishes, > > > Mick. > > > > On 28/11/2021 23:40, raf wrote: > > That looks good (exec with argument list rather than interpolated string). > > Phew! Thanks for the vote of confidence. Yes, avoiding the need to use /bin/sh is a big win. > > Minor Perl quibbles (Sorry, couldn't help myself): > I don't mind one bit. I only speak for myself mind you. > > > > > The comma before "or die" can be removed. > > I will remove it. > > > > Instead of the global file handle POSTMAP, it might be better to use "my > > $postmapfh" > > I will test that out tomorrow (er, today) in the afternoon after some sleep. > I'd rather future poof it now, than have to work out what went wrong later > later. Thank you for the tip. > > > > > Just checking: Is each line of output from POSTMAP supposed to replace > > any previous output, or should it append to it? > > Postmap only outputs one line per query. Either it replies with a result or > NULL. Maybe Of course. I was being dumb. > while (<POSTMAP>) { $vsresult= "$_"; } > > could be replaced with > > $vsresult= "$_"; > > ?? I will test. No real point in looping if only one line is returnd. Actually, postmap will output zero lines if the lookup fails, or one line if it succeeds, so the loop does do something. If the lookup fails, the while loop body won't execute, and the function will return "". If the lookup succeeds, the while loop body will execute, and the function will return the lookup result. So it's good as it is. One last bit of advice, add "use strict;" and "use warnings;" to the script and add "my" before the first use of each variable. You know you want to. :-) > Thanks, > Mick. cheers, raf
