Beefy Boxes and Bandwidth Generously Provided by pair Networks
good chemistry is complicated,
and a little bit messy -LW
 
PerlMonks  

untainting or encoding for shelled sqlplus update

by goibhniu (Hermit)
on May 15, 2008 at 18:40 UTC ( #686792=perlquestion: print w/ replies, xml ) Need Help??
goibhniu has asked for the wisdom of the Perl Monks concerning the following question:

I have a piece of "Other People's Code" that I'm maintaining. They updates Oracle by shelling out to sqlplus. It's not encoding the field values. If I try to update a field with a value that contains, for instance, a single-quote ('), it messes up the sql statement.

I'd like to not refactor to use DBI if I don't have to. What library should I grab from CPAN to do the encoding? Should I be searching for phrases like "encoding" Or "untainting"?

Sorry I don't have a code snippet, the place where the input is validated is several function calls away from the place where the sqlplus command is shelled. If needed, I'll work on an example.


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

Comment on untainting or encoding for shelled sqlplus update
Re: untainting or encoding for shelled sqlplus update
by moritz (Cardinal) on May 15, 2008 at 18:50 UTC
    You really should try to do as much as possible through DBI.

    Even if you don't manipulate the data you can use DBI's ->quote and ->quote_identifier methods to do the escaping.

      quote and quote_identifier are both database handle methods. Which means that you'd have to load DBI (and the appropriate DBD) to get the database handle to call these methods. So, going this route, the OP might as well refactor to just use DBI instead of the shell. Or, if for some reason, he can't use or install DBI, then at least untaint the data and escape the characters that you're willing to accept that need escaping.
        quote and quote_identifier are both database handle methods.

        They have to be, because escaping is handled differently in the various DBs out there. But it's the only safe method that I know of, which is why I recommend it, and recommend refactoring as much as possible at the same time.

        If you know another secure methods feel free to offer it.

        You don't know how much code he would have to refactor to make it all usable in DBI. It might be a day's worth of work, or a month.

        Creating a DBI handle will only take a few minutes and can be a test account. This is certainly easier, for a quick fix, than rewriting the whole thing to use DBI.

Re: untainting or encoding for shelled sqlplus update
by goibhniu (Hermit) on May 15, 2008 at 20:26 UTC

    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.'; } }

    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; } }
    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'"; . . .
    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'"; . . .
    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.

      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; }

        ++ 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.
Re: untainting or encoding for shelled sqlplus update
by goibhniu (Hermit) on May 19, 2008 at 19:21 UTC

    I have a solution. For those interested, all my attempts to escape special characters were slightly off target. If I ran the script in a DB IDE like SQL Navigator, it worked fine. The only time it failed was in sqlplus. It turns out that when sqlplus is processing multiline info, it treats an empty line sorta like the end of a <<HERE doc.

    I've left in all the special character encoding (for sql insertion reasons), but the thing that really fixed my script was:

    $string =~ s/\n\n/\n'||CHR(10)||'/g;

    and thanks to the guys in the Chatterbox (Intrepid, jdporter, mwah, bart, ambrus) for helping me write the regexp and consider corner cases on this.


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

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://686792]
Approved by moritz
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others avoiding work at the Monastery: (14)
As of 2014-07-22 21:02 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    My favorite superfluous repetitious redundant duplicative phrase is:









    Results (128 votes), past polls