| from Oracle8iDialect:374
@Override
public String getSelectSequenceNextValString(String sequenceName) {
return sequenceName + ".nextval";
}
this means that if you write code like this if your sequence name comes from user input it's vulnerable to sql injection. Here's a partial proof of concept (from our code) I can write a full one if necessary.
int getNextCodeIntegerFromSequence( final TestClassification classification )
{
Long next = entityManager.getSession()
.doReturningWork( conn ->
{
DatabaseMetaDataDialectResolutionInfoAdapter info
= new DatabaseMetaDataDialectResolutionInfoAdapter( conn.getMetaData() );
Dialect dialect = new StandardDialectResolver().resolveDialect( info );
String seq = classification.getCodeSequence();
/* if ( StringUtils.containsAny( seq, dialect.openQuote(), dialect.closeQuote() )) {
String msg = String.format( "classification is being nefarious: '%s'", classification );
throw new IllegalArgumentException( msg );
}
*/
String quoted = dialect.quote( "`" + seq + "`" );
String sql = dialect.getSequenceNextValString( quoted );
try ( PreparedStatement stmt = conn.prepareStatement( sql );
ResultSet res = stmt.executeQuery() )
{
while ( res.next() )
{
return res.getLong( 1 );
}
}
String fmt = "something went wrong, you shouldn't reach this, here's the "
+ "classifcation: %s";
throw new NoResultException( String.format( fmt, classification ) );
} );
return next.intValue();
}
and a test
@Test( expected = IllegalArgumentException.class )
public void getNextCodeIntegerFromSequenceExploit() {
TestClassification classification = testClassificationRepo.findOne( 1L );
classification.setCodeSequence( classification.getCodeSequence() + "\"; drop table site_user; --" );
codeDao.getNextCodeIntegerFromSequence( classification );
Integer siteUserTables = codeDao.getEntityManager().getSession()
.doReturningWork( conn ->
{
String sql
= " SELECT COUNT (TABLE_NAME ) from information_schema.tables "
+ " WHERE table_name = ? ";
try ( PreparedStatement stmt = conn.prepareStatement( sql ) )
{
stmt.setString( 1, "SITE_USER" );
try ( ResultSet res = stmt.executeQuery() )
{
while ( res.next() )
{
return res.getInt( 1 );
}
}
return null;
}
} );
assertThat( siteUserTables, greaterThan( 0 ) );
}
it should probably be implemented as
@Override
public String getSelectSequenceNextValString(String sequenceName) {
return quote( sequenceName ) + ".nextval";
}
of course then looking at the implementation of Dialect.quote, it wouldn't actually do anything. After reviewing, oracle, h2, and postgres docs (pg says this)
Note that dollar signs are not allowed in identifiers according to the letter of the SQL standard, so their use might render applications less portable.
I think that the quoting character for an identifier is never allowed in an identifier. So I think this can be added to the beginning of quote. StringUtils is from apache commons lang3
if ( StringUtils.containsAny( name, this.openQuote(), this.closeQuote() )) {
String msg = String.format( "illegal characters in: '%s'", name );
throw new IllegalArgumentException( msg );
}
If this plan is approved I can write a patch, including one that doesn't use commons lang 3 |
