commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hope, Matthew" <Matthew.H...@capitalone.com>
Subject RE: Checking if a String is both HTML and SQL safe
Date Fri, 22 Aug 2003 09:32:36 GMT
As already pointed out the latest builds of lang contain the funtionality
you desire however if you are allowing users to execute arbitrary sql then
escaping may not be sufficient for security especially given the various SQL
implementations the different vendors have.

About the only simple cast iron way to allow arbitrary user specified
variables is to use PreparedStatements (though this is more effort if you
wish to use arbitrary column names) since the database driver is being
relied on to spot issues (of course if your driver has problems your really
in trouble).

generally making sure the input is only of a certain length will prevent any
real nasties (though they could cause the query to keep crashing out and may
expose a resource closure flaw and kill your server)

for a single table select something along the lines of this would work but
is still not perfect:
class WhereClause
{
    private String columnName;
    private Object value;
	// plus get / set

    public void setValue(PreparedStatement ps, int index)
    {
	    // allows overriding to use setInt() setString(), setLong()
etc...
	   ps.setObject(index, value);
    }

    public String buildQuery()
    {
        if (isNull()) {
            return columnName + " is null";
        }
        return columnName + " = ?";
    }

    public boolean isNull()
    {
        return value == null;
    }
}

class SafeSQL
{
// assumes a pre loaded set of allowed table names allowedTables
    public ResultSet executeSelectWithUntrustedParameters(String tableName,
String[] columnNames, WhereClause[] where)
        throws SQLException
    {
        if (!allowedTables.contains(tableName)) {
            throw new SecurityException("table '" + tableName + "' not a
valid table!");
        }
        /* assume these variables exist and will be closed correctly when
finished with
Connection con;
PreparedStatement ps;
*/
        Set validColumnNames = getValidColumnNames(tableName, con);
        String cleanSQL = "select ";// you'd us a buffer but this is quicker
to type
        for (int columnIndex = 0; columnIndex < columnNames.length;
columnIndex++) {
            if (!validColumnNames.contains(columnNames[columnIndex])) {
                throw new SecurityException("table '" + tableName + "' does
not contain column " + columnNames);
            }
            if (columnIndex > 0) {
                cleanSQL += ", ";
            }
            cleanSQL += columnName;
        }
        cleanSQL += " from " + tableName;
// if where is empty this might allow a DoS attack - may wish to enforce
certain entries
        for (int whereIndex = 0; whereIndex < where.length; whereIndex++) {
            if (!validColumnNames.contains(where[whereIndex])) {
                throw new SecurityException("table '" + tableName + "' does
not contain column " + columnNames);
            }
            if (whereIndex == 0) {
                cleanSQL += " where ";
            } else {
                cleanSQL += " and ";
            }
            cleanSQL += where[whereIndex].buildQuery();
        }
        ps = con.prepareStatement(cleanSQL);
        int bindIndex = 1;
        for (whereIndex = 0; whereIndex < where.length; whereIndex++) {
            if (!where[whereIndex].isNull()) {
                where[whereIndex].setValue(ps, bindIndex++);
            }
        }
        return ps.executeQuery();
    }

// you'd cache this info
    private Set getValidColumnNames(String tableName, Connection con)
    {
        /* do something like
"select * from "+ tableName;
use ResultSetMetaData to get the column names out
and put into a set
*/
    }
}

This however if far from pleasant and doesn't support joins.

You will be much better if the user can only select from a list of columns
you supply (and revalidate on request) on
a specific table. then required where clauses and such like can more
effectively be supplied not to mention compile time type safe checks...

It should be farily clear that you really don't want to be trying to supply
such flexibility if you wish to keep things secure, much better to use one
of the available Object presistance layers available...

Matt
 
**************************************************************************
The information transmitted herewith is sensitive information intended only
for use by the individual or entity to which it is addressed. If the reader
of this message is not the intended recipient, you are hereby notified that
any review, retransmission, dissemination, distribution, copying or other
use of, or taking of any action in reliance upon this information is
strictly prohibited. If you have received this communication in error,
please contact the sender and delete the material from your computer.

Mime
View raw message