from {{Oracle8iDialect}}:374
{code}
@Override
public String getSelectSequenceNextValString(String sequenceName) {
return sequenceName + ".nextval";
}
{code}
this means that if you write code like [this|http://stackoverflow.com/a/23267212/206466] 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.
{code :java }
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();
}
{code}
and a test
{code :java }
@Test( expected = IllegalArgumentException.class )
public void getNextCodeIntegerFromSequenceExploit() {
TestClassification classification = testClassificationRepo.findOne( 1L );
classification.setCodeSequence( classification.getCodeSequence() + "\"; drop table site_user; --" );
codeDao.getNextCodeIntegerFromSequence( classification ); // throws, code below never reached but left for
// proof of concept testing
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 ) );
}
{code}
it should probably be implemented as
{code}
@Override
public String getSelectSequenceNextValString(String sequenceName) {
return quote( sequenceName ) + ".nextval";
}
{code}
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)
{quote}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.
{quote}
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
{code}
if ( StringUtils.containsAny( name, this.openQuote(), this.closeQuote() )) {
String msg = String.format( "illegal characters in: '%s'", name );
throw new IllegalArgumentException( msg );
}
{code}
If this plan is approved I can write a patch, including one that doesn't use commons lang 3 |
|
