Alternatives to Evaluate

Question

An audit has shown up a vulnerability in our use of evaluate as it allows arbitrary code execution. Has anyone got an alternative? Example below. We're running CF9.

<cfquery name="getvalue" datasource="#application.ds#">
   SELECT     #url.column#
   FROM         dbo.tbl#url.table#
   WHERE     (int#url.table#Id = <CFQUERYPARAM Value="#url.Id#">)
</cfquery>

<cfif url.rowtype eq "text">
    <cfoutput>
    <input type="text" id="focus#url.table##url.column##url.Id#" 
        name="#url.table##url.column##url.Id#" 
        value="#evaluate('getvalue.#url.column#')#" 
        class="inputtext"
        onblur="updateeditvalue('#url.rowtype#','#url.table#','#url.column#',#url.Id#
                    ,escape(this.form.#url.table##url.column##url.Id#.value))" 
        style="height:20px;width:500px;">
    </cfoutput>
<cfelseif url.rowtype eq "textarea">
     <cfoutput>
     <textarea id="focus#url.table##url.column##url.Id#" 
        name="#url.table##url.column##url.Id#" 
        class="inputtext" style="height:20px;width:500px;"
        onblur="updateeditvalue('#url.rowtype#','#url.table#','#url.column#',#url.Id#
                     , escape(this.form.#url.table##url.column##url.Id#.value))">
          #evaluate('getvalue.#url.column#')#
    </textarea>
    </cfoutput>
</cfif>

Also, you need to use [HtmlEditFormat](http://cfdocs.org/htmleditformat) and [JsStringFormat](http://cfdocs.org/jsstringformat) to prevent HTML/JS injection — Peter Boughton, Oct 12 '12 at 11:55
AND you've got SQL injection there with all the #url.stuff# in the query. — Peter Boughton, Oct 12 '12 at 11:55
I'm not sure if I can avoid the #url.stuff# - the page is dynamic and we have to look up the column, table etc. What would you suggest? — Jake Langford, Oct 12 '12 at 12:01
Make sure it's a known/expected value - AlexP shows one option below, but you can also use a whitelist (e.g. with ListFind). However, the very fact that you've got dynamic column, table _and_ id names does raise flags - what are you actually trying to do here? If it's some sort of database admin system, there's probably an existing tool that solves the problem and has addressed the security issues already. — Peter Boughton, Oct 12 '12 at 12:31

score 1 · Answer 1 · answered Oct 12 '12 at 12:01

1

you will need to use getvalue[url.column][1] so you are pulling in row 1 of the getvalue query, you'll get an error otherwise. Peter is correct about SQL injection issues as well. At a minimum you need to use <cfqueryparam> for your queries

answered Oct 12 '12 at 12:01

Matt Busche

14,216
5
36
61

even if there's only one row possible? – Jake Langford Oct 12 '12 at 12:06

score 1 · Answer 2 · answered Oct 12 '12 at 12:06

1

You shouldn't be including the column/table/id column names directly within the URL, this is opening up the SQL for injection. I see you have used the cfqueryparam for the value, however the rest of the query makes this redundant.

You should pass aliases via the URL and then set the correct column names. A very quick example is below:

<cfset idColumn   = ""/>
<cfset columnName = ""/>
<cfset tableName  = ""/>

<cfif structKeyExists(url, "aliasForColumnA")>
  <cfset columnName = "the_real_column_name_a"/>
  <cfset tableName  = "the_real_table_name_a"/>
  <cfset idColumn   = "int" & #tableName# & "Id"/>
<cfelseif structKeyExists(url, "aliasForColumnB")>
  <cfset columnName = "the_real_column_name_b"/>
  <cfset tableName  = "the_real_table_name_b"/>
  <cfset idColumn   = "int" & #tableName# & "Id"/>
</cfif>

<cfquery name="getvalue" datasource="#application.ds#">
  SELECT
    #columnName#
  FROM
    #tableName#
  WHERE
    #idColumn# = <CFQUERYPARAM cfsqltype="CF_SQL_INTEGER" Value="#url.Id#"/>
</cfquery>

answered Oct 12 '12 at 12:06

AlexP

9,906
1
24
43

1

This code currently executes the query even if the values are blank - needs to check for failure. Though for easier maintainability I'd probably do it slightly different - e.g. create the alias data as a struct (or dynamically generate) then have a single url.alias field... – Peter Boughton Oct 12 '12 at 12:37
1

e.g: `` ` ` – Peter Boughton Oct 12 '12 at 12:37
Yes I agree, this was a `A very quick example` just wanted to make my point with regards to the injection side of things and offer a possible solution. – AlexP Oct 12 '12 at 15:37
Another benefit to using aliases is you avoid exposing your actual db schema. – Leigh Oct 12 '12 at 17:02

score 1 · Answer 3 · answered Oct 12 '12 at 14:10

If you really have to run SQL that is that dynamic, here's what I'd do. onApplicationStart you query the database metadata using cfdbinfo for a list of tables in your database and store them in application scope.

You can write a function which takes URL.table and checks that it exists in your list. It can throw an error if it's not listed.

The same approach would work for columns. That'll check that your tables/columns exist, but if you want to only allow access to certain tables/columns then you're no choice to store some kind of list or set of aliases as AlexP and Peter suggest.

Alternatives to Evaluate

3 Answers3