From 0a7b54e0313d840af3dc21fa122f7707c8b35c20 Mon Sep 17 00:00:00 2001 From: zhangtao2020 <18066722603@163.com> Date: Sat, 12 Dec 2020 18:40:37 +0800 Subject: [PATCH] CVE-2020-25638 --- .../java/org/hibernate/dialect/Dialect.java | 11 ++ .../internal/SelectStatementBuilder.java | 2 +- .../main/java/org/hibernate/sql/Delete.java | 3 +- .../main/java/org/hibernate/sql/Insert.java | 2 +- .../java/org/hibernate/sql/InsertSelect.java | 2 +- .../java/org/hibernate/sql/QuerySelect.java | 2 +- .../main/java/org/hibernate/sql/Select.java | 2 +- .../java/org/hibernate/sql/SimpleSelect.java | 2 +- .../main/java/org/hibernate/sql/Update.java | 2 +- .../hibernate/test/comments/TestEntity.java | 46 ++++++++ .../hibernate/test/comments/TestEntity2.java | 37 ++++++ .../test/comments/UseSqlCommentTest.java | 111 ++++++++++++++++++ 12 files changed, 214 insertions(+), 8 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/comments/TestEntity.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/comments/TestEntity2.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/comments/UseSqlCommentTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index fd286e3..8d3ded8 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java @@ -24,6 +24,7 @@ import java.util.Locale; import java.util.Map; import java.util.Properties; import java.util.Set; +import java.util.regex.Pattern; import org.hibernate.HibernateException; import org.hibernate.LockMode; @@ -140,6 +141,9 @@ public abstract class Dialect implements ConversionContext { */ public static final String CLOSED_QUOTE = "`\"]"; + private static final Pattern ESCAPE_CLOSING_COMMENT_PATTERN = Pattern.compile( "\\*/" ); + private static final Pattern ESCAPE_OPENING_COMMENT_PATTERN = Pattern.compile( "/\\*" ); + private final TypeNames typeNames = new TypeNames(); private final TypeNames hibernateTypeNames = new TypeNames(); @@ -2866,4 +2870,11 @@ public abstract class Dialect implements ConversionContext { public boolean supportsNamedParameters(DatabaseMetaData databaseMetaData) throws SQLException { return databaseMetaData != null && databaseMetaData.supportsNamedParameters(); } + public static String escapeComment(String comment) { + if ( StringHelper.isNotEmpty( comment ) ) { + final String escaped = ESCAPE_CLOSING_COMMENT_PATTERN.matcher( comment ).replaceAll( "*\\\\/" ); + return ESCAPE_OPENING_COMMENT_PATTERN.matcher( escaped ).replaceAll( "/\\\\*" ); + } + return comment; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/query/internal/SelectStatementBuilder.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/query/internal/SelectStatementBuilder.java index 3d2b1d8..41ebbab 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/query/internal/SelectStatementBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/query/internal/SelectStatementBuilder.java @@ -187,7 +187,7 @@ public class SelectStatementBuilder { StringBuilder buf = new StringBuilder( guesstimatedBufferSize ); if ( StringHelper.isNotEmpty( comment ) ) { - buf.append( "/* " ).append( comment ).append( " */ " ); + buf.append( "/* " ).append( Dialect.escapeComment(comment)).append( " */ " ); } buf.append( "select " ) diff --git a/hibernate-core/src/main/java/org/hibernate/sql/Delete.java b/hibernate-core/src/main/java/org/hibernate/sql/Delete.java index a6badc0..3bd50df 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/Delete.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/Delete.java @@ -8,6 +8,7 @@ package org.hibernate.sql; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; +import org.hibernate.dialect.Dialect; /** * An SQL DELETE statement @@ -36,7 +37,7 @@ public class Delete { public String toStatementString() { StringBuilder buf = new StringBuilder( tableName.length() + 10 ); if ( comment!=null ) { - buf.append( "/* " ).append(comment).append( " */ " ); + buf.append( "/* " ).append( Dialect.escapeComment( comment ) ).append( " */ " ); } buf.append( "delete from " ).append(tableName); if ( where != null || !primaryKeyColumns.isEmpty() || versionColumnName != null ) { diff --git a/hibernate-core/src/main/java/org/hibernate/sql/Insert.java b/hibernate-core/src/main/java/org/hibernate/sql/Insert.java index 49d828c..ed15af8 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/Insert.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/Insert.java @@ -90,7 +90,7 @@ public class Insert { public String toStatementString() { StringBuilder buf = new StringBuilder( columns.size()*15 + tableName.length() + 10 ); if ( comment != null ) { - buf.append( "/* " ).append( comment ).append( " */ " ); + buf.append( "/* " ).append( Dialect.escapeComment( comment ) ).append( " */ " ); } buf.append("insert into ") .append(tableName); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/InsertSelect.java b/hibernate-core/src/main/java/org/hibernate/sql/InsertSelect.java index 387ee1d..a8038e6 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/InsertSelect.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/InsertSelect.java @@ -65,7 +65,7 @@ public class InsertSelect { StringBuilder buf = new StringBuilder( (columnNames.size() * 15) + tableName.length() + 10 ); if ( comment!=null ) { - buf.append( "/* " ).append( comment ).append( " */ " ); + buf.append( "/* " ).append( Dialect.escapeComment(comment)).append( " */ " ); } buf.append( "insert into " ).append( tableName ); if ( !columnNames.isEmpty() ) { diff --git a/hibernate-core/src/main/java/org/hibernate/sql/QuerySelect.java b/hibernate-core/src/main/java/org/hibernate/sql/QuerySelect.java index 649d973..7d45a3e 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/QuerySelect.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/QuerySelect.java @@ -126,7 +126,7 @@ public class QuerySelect { public String toQueryString() { StringBuilder buf = new StringBuilder( 50 ); if ( comment != null ) { - buf.append( "/* " ).append( comment ).append( " */ " ); + buf.append( "/* " ).append( Dialect.escapeComment(comment)).append( " */ " ); } buf.append( "select " ); if ( distinct ) { diff --git a/hibernate-core/src/main/java/org/hibernate/sql/Select.java b/hibernate-core/src/main/java/org/hibernate/sql/Select.java index e8b0aa4..cf1e6e3 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/Select.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/Select.java @@ -40,7 +40,7 @@ public class Select { public String toStatementString() { StringBuilder buf = new StringBuilder(guesstimatedBufferSize); if ( StringHelper.isNotEmpty(comment) ) { - buf.append("/* ").append(comment).append(" */ "); + buf.append( "/* " ).append( Dialect.escapeComment(comment)).append( " */ " ); } buf.append("select ").append(selectClause) diff --git a/hibernate-core/src/main/java/org/hibernate/sql/SimpleSelect.java b/hibernate-core/src/main/java/org/hibernate/sql/SimpleSelect.java index 51fbc86..402cd01 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/SimpleSelect.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/SimpleSelect.java @@ -143,7 +143,7 @@ public class SimpleSelect { ); if ( comment != null ) { - buf.append( "/* " ).append( comment ).append( " */ " ); + buf.append( "/* " ).append( Dialect.escapeComment(comment)).append( " */ " ); } buf.append( "select " ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/Update.java b/hibernate-core/src/main/java/org/hibernate/sql/Update.java index 93611d7..c080400 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/Update.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/Update.java @@ -166,7 +166,7 @@ public class Update { public String toStatementString() { StringBuilder buf = new StringBuilder( (columns.size() * 15) + tableName.length() + 10 ); if ( comment!=null ) { - buf.append( "/* " ).append( comment ).append( " */ " ); + buf.append( "/* " ).append( Dialect.escapeComment(comment)).append( " */ " ); } buf.append( "update " ).append( tableName ).append( " set " ); boolean assignmentsAppended = false; diff --git a/hibernate-core/src/test/java/org/hibernate/test/comments/TestEntity.java b/hibernate-core/src/test/java/org/hibernate/test/comments/TestEntity.java new file mode 100644 index 0000000..7c425be --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/comments/TestEntity.java @@ -0,0 +1,46 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.test.comments; + +import javax.persistence.Entity; +import javax.persistence.Id; + +/** + * @author Andrea Boriero + */ +@Entity +public class TestEntity { + @Id + private String id; + + private String value; + + public TestEntity() { + + } + + public TestEntity(String id, String value) { + this.id = id; + this.value = value; + } + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/comments/TestEntity2.java b/hibernate-core/src/test/java/org/hibernate/test/comments/TestEntity2.java new file mode 100644 index 0000000..58b626d --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/comments/TestEntity2.java @@ -0,0 +1,37 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.test.comments; + +import javax.persistence.Entity; +import javax.persistence.Id; + +/** + * @author Andrea Boriero + */ +@Entity +public class TestEntity2 { + @Id + private String id; + + private String value; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/comments/UseSqlCommentTest.java b/hibernate-core/src/test/java/org/hibernate/test/comments/UseSqlCommentTest.java new file mode 100644 index 0000000..2bd6adf --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/comments/UseSqlCommentTest.java @@ -0,0 +1,111 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.test.comments; + +import java.util.List; +import java.util.Map; +import javax.persistence.EntityManager; +import javax.persistence.TypedQuery; +import javax.persistence.criteria.CompoundSelection; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.Path; +import javax.persistence.criteria.Root; + +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase; + +import org.junit.Before; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.hibernate.testing.transaction.TransactionUtil.doInJPA; +import static org.junit.Assert.assertThat; + +/** + * @author Andrea Boriero + */ +public class UseSqlCommentTest extends BaseEntityManagerFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { TestEntity.class, TestEntity2.class }; + } + + @Override + protected void addMappings(Map settings) { + settings.put( AvailableSettings.USE_SQL_COMMENTS, "true" ); + settings.put( AvailableSettings.FORMAT_SQL, "false" ); + } + + @Before + public void setUp() { + doInJPA( this::entityManagerFactory, entityManager -> { + TestEntity testEntity = new TestEntity(); + testEntity.setId( "test1" ); + testEntity.setValue( "value1" ); + entityManager.persist( testEntity ); + + TestEntity2 testEntity2 = new TestEntity2(); + testEntity2.setId( "test2" ); + testEntity2.setValue( "value2" ); + entityManager.persist( testEntity2 ); + } ); + } + + @Test + public void testIt() { + String appendLiteral = "*/select id as col_0_0_,value as col_1_0_ from testEntity2 where 1=1 or id=?--/*"; + doInJPA( this::entityManagerFactory, entityManager -> { + + List result = findUsingQuery( "test1", appendLiteral, entityManager ); + + TestEntity test1 = result.get( 0 ); + assertThat( test1.getValue(), is( appendLiteral ) ); + } ); + + doInJPA( this::entityManagerFactory, entityManager -> { + + List result = findUsingCriteria( "test1", appendLiteral, entityManager ); + + TestEntity test1 = result.get( 0 ); + assertThat( test1.getValue(), is( appendLiteral ) ); + } ); + } + + public List findUsingCriteria(String id, String appendLiteral, EntityManager entityManager) { + CriteriaBuilder builder = entityManager.getCriteriaBuilder(); + CriteriaQuery criteria = builder.createQuery( TestEntity.class ); + Root root = criteria.from( TestEntity.class ); + + Path idPath = root.get( "id" ); + CompoundSelection selection = builder.construct( + TestEntity.class, + idPath, + builder.literal( appendLiteral ) + ); + criteria.select( selection ); + + criteria.where( builder.equal( idPath, builder.parameter( String.class, "where_id" ) ) ); + + TypedQuery query = entityManager.createQuery( criteria ); + query.setParameter( "where_id", id ); + return query.getResultList(); + } + + public List findUsingQuery(String id, String appendLiteral, EntityManager entityManager) { + TypedQuery query = + entityManager.createQuery( + "select new org.hibernate.test.comments.TestEntity(id, '" + + appendLiteral.replace( "'", "''" ) + + "') from TestEntity where id=:where_id", + TestEntity.class + ); + query.setParameter( "where_id", id ); + return query.getResultList(); + } +} -- 2.27.0