in reply to untainting or encoding for shelled sqlplus update

Thanks for everyone's input. I know "what's right" is to refactor to DBI. I'm hoping to get away with less. Here's an attempt at code examples to give you all a better idea at how much refactoring might be involved in various designs.

I call it here: 

        $result = update_string_to_form($reportoutput, $thisformobjid,
+ 'REPORTBODY')    ;
        if (not defined $result) {
            die 'could not update report body on form';
        } else {
            if ($result != 0) {
                die 'error updating report body on form.';
            }
        }

[download]

This is update_string_to_form: 

sub update_string_to_form {
    my ($string, $formobjid, $column) = @_;
    return undef unless defined $string;      #string is required to b
+e defined, but can be ''
    return undef unless $formobjid;           #formobjid is required
    return undef unless $column;              #column is required
    unless ( grep($_ eq $column, formcolumns_from_formobjid($formobjid
+)) ){
        warn ("invalid column for form");
        return undef;
    }
    my $formtypeobjid = formtypeobjid_from_formobjid($formobjid);
    return undef unless $formtypeobjid;
    my $formtablename = formtablename_from_formtypeobjid($formtypeobji
+d);
    return undef unless $formtablename;
    my $sql = "update $formtablename set $column = '$string' where FOR
+MOBJID = '$formobjid'";
    #if ( harcommon::debug() ) {
    if ( 1 ) {
        warn '$formobjid      : ',$formobjid;
        warn '$formtypeobjid  : ',$formtypeobjid;
        warn '$formtablename  : ',$formtablename;
        warn '$string         : ',$string;
        warn '$sql            : ',$sql;
    }
    my @results = 
       harcommon::runsql( $sql, $$DBSettings{'tnsname'}, $$DBSettings{
+'dbuser'}, $$DBSettings{'dbpass'} );
    #if ( harcommon::debug() ) {
    if ( 1 ) {
       warn '@results: ',join(",",@results);
       warn 'number of results: ', scalar(@results);
    }
    if (scalar(@results) != 0) {
       my @trimmedresults = map {harcommon::trim($_)} @results;
       return @trimmedresults;
    } else {
       return 0;
    }
}

[download]
and somewhere down in harcommon::runsql, they encapsulate looking up credentials and wrapping my stuff in a sqlplus command and executing / catching results from sqlplus.

My first naive solution was to add: 

 . . .
    $string =~ s/'/quot/g;
    $string =~ s/;/semi/g;
    my $sql = "update $formtablename set $column = '$string' where FOR
+MOBJID = '$formobjid'";

 . . .

[download]
before setting $sql. FWIW, this approach is still failing (though I'm debugging and looking for dumb mistakes).

One design (moritz's suggestion, which I like) would be to use the minimum of DBI to get access to ->quote. 

 . . .
    my $dbh = DBI->connect($data_source, $username, $auth, \%attr);
    $string = $dbh->quote($string);
    my $sql = "update $formtablename set $column = '$string' where FOR
+MOBJID = '$formobjid'";
 . . .

[download]
but now I would have to un-encapsulate the looking up of $data_source, $username, $auth, etc. from the harcommon::runsql sub just to pass $sql in and let it do that again.

I suppose the next thing would be to refactor harcommon::runsqlto use DBI and abandon sqlplus entirely. That's getting to be orders of magnitude larger a problem than I thought I was working on.

#my sig used to say 'I humbly seek wisdom. '. Now it says:
use strict;
use warnings;
I humbly seek wisdom.

Replies are listed 'Best First'.
Re^2: untainting or encoding for shelled sqlplus update
by tachyon-II (Chaplain) on May 15, 2008 at 21:05 UTC

    Using DBI makes good sense. I don't use Oracle so can't test however according to this and other Google hits something like this may work. You may need to send a "set escape \" command to SQLPLUS too. 

    my @res_word = qw( ABOUT ACCUM AND BT BTG BTI BTP FUZZY HASPATH 
                   INPATH MINUS NEAR NOT NT NTG NTI NTP OR PT RT 
                   SQE SYN TR TRSYN TT WITHIN );
my @res_char = qw( , & ? { } \ ( ) [ ] - ; ~ | $ ! > * % _ );
my $rw = join '|', @res_word;
$rw = qr/$rw/;
my $rc = join '', map{"\\$_"}@res_char;
$rc = qr/[$rc]/;

sub escape {
    my $str = shift;
    $str =~ s/($rc)/\\$1/g;  # reserved char escapes
    $str =~ s/($rw)/{$1}/g;  # reserved word escapes
    $str =~ s/(['"])/$1$1/g; # quote escapes
  return $str;
}

    [download]
[reply]
[d/l]

      ++ I obviously don't work directly with the DB enough either, but your example and reference give me a more exhaustive list of chars, etc. to untaint. That may do just the trick for now (though ikegami's comment convinces me that using the DBI quote function is the right thing).

      #my sig used to say 'I humbly seek wisdom. '. Now it says:
      use strict;
      use warnings;
      I humbly seek wisdom.
[reply]

        While a quick kludge may fix your problem with minimal effort using the bulk loader functionality that comes bundled with oracle makes better sense (speed and correct quoting) than using DBI to me.

[reply]

In Section Seekers of Perl Wisdom