Return-Path: X-Original-To: apmail-portals-jetspeed-dev-archive@www.apache.org Delivered-To: apmail-portals-jetspeed-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 80AFC18CF9 for ; Tue, 26 Jan 2016 03:38:17 +0000 (UTC) Received: (qmail 6637 invoked by uid 500); 26 Jan 2016 03:38:17 -0000 Delivered-To: apmail-portals-jetspeed-dev-archive@portals.apache.org Received: (qmail 6448 invoked by uid 500); 26 Jan 2016 03:38:17 -0000 Mailing-List: contact jetspeed-dev-help@portals.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Jetspeed Developers List" Delivered-To: mailing list jetspeed-dev@portals.apache.org Received: (qmail 6426 invoked by uid 99); 26 Jan 2016 03:38:16 -0000 Received: from Unknown (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 26 Jan 2016 03:38:16 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id 7E9251A0469 for ; Tue, 26 Jan 2016 03:38:16 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 0.446 X-Spam-Level: X-Spam-Status: No, score=0.446 tagged_above=-999 required=6.31 tests=[KAM_LAZY_DOMAIN_SECURITY=1, RP_MATCHES_RCVD=-0.554] autolearn=disabled Received: from mx1-us-east.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id XsVE6zXgWNv0 for ; Tue, 26 Jan 2016 03:38:13 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-us-east.apache.org (ASF Mail Server at mx1-us-east.apache.org) with ESMTP id 2C5E043B3C for ; Tue, 26 Jan 2016 03:38:09 +0000 (UTC) Received: from svn01-us-west.apache.org (svn.apache.org [10.41.0.6]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 7D3F9E047F for ; Tue, 26 Jan 2016 03:38:08 +0000 (UTC) Received: from svn01-us-west.apache.org (localhost [127.0.0.1]) by svn01-us-west.apache.org (ASF Mail Server at svn01-us-west.apache.org) with ESMTP id 5701F3A00E7 for ; Tue, 26 Jan 2016 03:38:08 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1726722 - /portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedPrincipalLookupManagerAbstract.java Date: Tue, 26 Jan 2016 03:38:08 -0000 To: jetspeed-dev@portals.apache.org From: woonsan@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20160126033808.5701F3A00E7@svn01-us-west.apache.org> Author: woonsan Date: Tue Jan 26 03:38:08 2016 New Revision: 1726722 URL: http://svn.apache.org/viewvc?rev=1726722&view=rev Log: convert sql statement manipulations to prepared statements to avoid any potential vulnerability Modified: portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedPrincipalLookupManagerAbstract.java Modified: portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedPrincipalLookupManagerAbstract.java URL: http://svn.apache.org/viewvc/portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedPrincipalLookupManagerAbstract.java?rev=1726722&r1=1726721&r2=1726722&view=diff ============================================================================== --- portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedPrincipalLookupManagerAbstract.java (original) +++ portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedPrincipalLookupManagerAbstract.java Tue Jan 26 03:38:08 2016 @@ -22,7 +22,12 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import javax.swing.text.Segment; import org.apache.jetspeed.security.JetspeedPrincipal; import org.apache.jetspeed.security.JetspeedPrincipalAssociationType; @@ -50,6 +55,11 @@ public abstract class JetspeedPrincipalL static final Logger log = LoggerFactory.getLogger(JetspeedPrincipalLookupManagerAbstract.class); + private static final String PARAM_PLACEHOLDER_PREFIX = "@@paramPlaceHolder"; + private static final String PARAM_PLACEHOLDER_SUFFIX = "@@"; + private static final Pattern PARAM_PLACEHOLDER_PATTERN = Pattern + .compile("(" + PARAM_PLACEHOLDER_PREFIX + "\\d+" + PARAM_PLACEHOLDER_SUFFIX + ")"); + /* * (non-Javadoc) * @@ -58,31 +68,29 @@ public abstract class JetspeedPrincipalL * getPrincipals(org.apache.jetspeed.security.JetspeedPrincipalQueryContext) */ public JetspeedPrincipalResultList getPrincipals(JetspeedPrincipalQueryContext queryContext) { - String baseSqlStr = this.generateBaseSql(queryContext); - - // create paging sql statement if possible for database based paging - String sqlStr = getPagingSql(baseSqlStr, queryContext); - int numberOfRecords = 0; ArrayList results = new ArrayList(); Connection conn = null; - PreparedStatement pstmt = null; - ResultSet rs = null; + + PreparedStatement pstmtForPaging = null; + ResultSet rsForPaging = null; + + PreparedStatement pstmtForCount = null; + ResultSet rsForCount = null; try { conn = PersistenceBrokerFactory.defaultPersistenceBroker().serviceConnectionManager().getConnection(); + PreparedStatement [] pstmts = createPagingPreparedStatementAndCountPreparedStatement(conn, queryContext); + pstmtForPaging = pstmts[0]; + pstmtForCount = pstmts[1]; - pstmt = conn.prepareStatement(sqlStr, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); - // pstmt = conn.prepareStatement(sqlStr, - // ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY); - pstmt.setFetchSize((int) (queryContext.getOffset() + queryContext.getLength())); - rs = pstmt.executeQuery(); - boolean hasRecords = rs.next(); + rsForPaging = pstmtForPaging.executeQuery(); + boolean hasRecords = rsForPaging.next(); if (hasRecords) { // scroll the result set to the offset - scrollToOffset(conn, rs, queryContext.getOffset()); + scrollToOffset(conn, rsForPaging, queryContext.getOffset()); for (int i = 0; i < queryContext.getLength(); i++) { // now materialize the ResultSet into a JetspeedPrincipal RowReader rr = PersistenceBrokerFactory.defaultPersistenceBroker().getClassDescriptor( @@ -90,38 +98,31 @@ public abstract class JetspeedPrincipalL Map row = new HashMap(); // TODO: optimize, just retrieve the id from the DB and setup // a JetspeedPrincipal template on that. - rr.readObjectArrayFrom(rs, row); + rr.readObjectArrayFrom(rsForPaging, row); PersistentJetspeedPrincipal p = (PersistentJetspeedPrincipal) rr.readObjectFrom(row); QueryByCriteria query = new QueryByCriteria(p); p = (PersistentJetspeedPrincipal) PersistenceBrokerFactory.defaultPersistenceBroker() .getObjectByQuery(query); results.add(p); - if (!rs.next()) { + if (!rsForPaging.next()) { break; } } - rs.close(); - rs = null; - pstmt.close(); - pstmt = null; - - // get the total number of results effected by the query - int fromPos = baseSqlStr.toUpperCase().indexOf(" FROM "); - if (fromPos >= 0) { - baseSqlStr = "select count(SECURITY_PRINCIPAL.PRINCIPAL_ID)" + baseSqlStr.substring(fromPos); - } - // strip ORDER BY clause - int orderPos = baseSqlStr.toUpperCase().indexOf(" ORDER BY "); - if (orderPos >= 0) { - baseSqlStr = baseSqlStr.substring(0, orderPos); + rsForPaging.close(); + rsForPaging = null; + pstmtForPaging.close(); + pstmtForPaging = null; + + rsForCount = pstmtForCount.executeQuery(); + while (rsForCount.next()) { + numberOfRecords += rsForCount.getInt(1); } - pstmt = conn.prepareStatement(baseSqlStr); - rs = pstmt.executeQuery(); - while (rs.next()) { - numberOfRecords += rs.getInt(1); - } + rsForCount.close(); + rsForCount = null; + pstmtForCount.close(); + pstmtForCount = null; } } catch (SQLException e) { log.error("Error reading principal.", e); @@ -130,21 +131,41 @@ public abstract class JetspeedPrincipalL } catch (LookupException e) { log.error("Error reading principal.", e); } finally { - if(rs != null) + if(rsForPaging != null) { try { - rs.close(); + rsForPaging.close(); } catch (Exception ignore) { } } - if(pstmt != null) + if(pstmtForPaging != null) { try { - pstmt.close(); + pstmtForPaging.close(); + } + catch (Exception ignore) + { + } + } + if(rsForCount != null) + { + try + { + rsForCount.close(); + } + catch (Exception ignore) + { + } + } + if(pstmtForCount != null) + { + try + { + pstmtForCount.close(); } catch (Exception ignore) { @@ -165,13 +186,247 @@ public abstract class JetspeedPrincipalL return new JetspeedPrincipalResultList(results, numberOfRecords); } - /** + private String putParamPlaceHolder(Map paramPlaceHolders, Object value) { + String paramPlaceHolderName = PARAM_PLACEHOLDER_PREFIX + paramPlaceHolders.size() + PARAM_PLACEHOLDER_SUFFIX; + paramPlaceHolders.put(paramPlaceHolderName, value); + return paramPlaceHolderName; + } + + private PreparedStatement[] createPagingPreparedStatementAndCountPreparedStatement(Connection conn, + JetspeedPrincipalQueryContext queryContext) throws SQLException { + String _paramPlaceHolderName = null; + Map _paramPlaceHolders = new HashMap(); + + String attributeConstraint = null; + String fromPart = "SECURITY_PRINCIPAL"; + + if (queryContext.getSecurityAttributes() != null) { + int cnt = 1; + + for (Map.Entry attribute : queryContext.getSecurityAttributes().entrySet()) { + if (attributeConstraint == null) { + _paramPlaceHolderName = putParamPlaceHolder(_paramPlaceHolders, attribute.getKey()); + attributeConstraint = " a" + cnt + ".PRINCIPAL_ID=SECURITY_PRINCIPAL.PRINCIPAL_ID AND a" + cnt + + ".ATTR_NAME = " + _paramPlaceHolderName; + + _paramPlaceHolderName = putParamPlaceHolder(_paramPlaceHolders, attribute.getValue()); + attributeConstraint += " AND a" + cnt + ".ATTR_VALUE LIKE " + _paramPlaceHolderName; + } else { + _paramPlaceHolderName = putParamPlaceHolder(_paramPlaceHolders, attribute.getKey()); + attributeConstraint += " AND a" + cnt + ".PRINCIPAL_ID=SECURITY_PRINCIPAL.PRINCIPAL_ID AND a" + cnt + + ".ATTR_NAME = " + _paramPlaceHolderName; + + _paramPlaceHolderName = putParamPlaceHolder(_paramPlaceHolders, attribute.getValue()); + attributeConstraint += " AND a" + cnt + ".ATTR_VALUE LIKE " + _paramPlaceHolderName; + } + + fromPart += ", SECURITY_ATTRIBUTE a" + cnt; + cnt++; + } + } + + String constraint = null; + + if (queryContext.getNameFilter() != null && queryContext.getNameFilter().length() > 0) { + _paramPlaceHolderName = putParamPlaceHolder(_paramPlaceHolders, + queryContext.getNameFilter().replace('*', '%')); + constraint = "SECURITY_PRINCIPAL.PRINCIPAL_NAME LIKE " + _paramPlaceHolderName; + } + + // find principals that are member of one or many roles + // the principal must be member in all supplied roles. + String roleConstraints = null; + + if (queryContext.getAssociatedRoles() != null && queryContext.getAssociatedRoles().size() > 0 + && queryContext.getAssociatedRoles().get(0).length() > 0) { + int cnt = 1; + + for (String roleName : queryContext.getAssociatedRoles()) { + _paramPlaceHolderName = putParamPlaceHolder(_paramPlaceHolders, roleName); + + if (roleConstraints == null) { + roleConstraints = "r" + cnt + ".ASSOC_NAME = '" + JetspeedPrincipalAssociationType.IS_MEMBER_OF + + "' " + "' AND r" + cnt + ".TO_PRINCIPAL_ID=rp" + cnt + ".PRINCIPAL_ID AND rp" + cnt + + ".PRINCIPAL_NAME LIKE " + _paramPlaceHolderName + " AND rp" + cnt + + ".PRINCIPAL_TYPE='role' AND r" + cnt + + ".FROM_PRINCIPAL_ID=SECURITY_PRINCIPAL.PRINCIPAL_ID"; + } else { + roleConstraints = " AND r" + cnt + ".ASSOC_NAME='" + JetspeedPrincipalAssociationType.IS_MEMBER_OF + + "' AND r" + cnt + ".TO_PRINCIPAL_ID=rp" + cnt + ".PRINCIPAL_ID AND rp" + cnt + + ".PRINCIPAL_NAME LIKE " + _paramPlaceHolderName + " AND rp" + cnt + + ".PRINCIPAL_TYPE='role' AND r" + cnt + + ".FROM_PRINCIPAL_ID=SECURITY_PRINCIPAL.PRINCIPAL_ID"; + } + } + + fromPart += ", SECURITY_PRINCIPAL_ASSOC r" + cnt + ", SECURITY_PRINCIPAL rp" + cnt; + cnt++; + } + + // find principals that are member of one or many groups + // the principal must be member in all supplied groups. + String groupConstraints = null; + + if (queryContext.getAssociatedGroups() != null && queryContext.getAssociatedGroups().size() > 0 + && queryContext.getAssociatedGroups().get(0).length() > 0) { + int cnt = 1; + + for (String groupName : queryContext.getAssociatedGroups()) { + _paramPlaceHolderName = putParamPlaceHolder(_paramPlaceHolders, groupName); + + if (groupConstraints == null) { + groupConstraints = "r" + cnt + ".ASSOC_NAME='" + JetspeedPrincipalAssociationType.IS_MEMBER_OF + + "' AND r" + cnt + ".TO_PRINCIPAL_ID=rp" + cnt + ".PRINCIPAL_ID AND rp" + cnt + + ".PRINCIPAL_NAME LIKE " + _paramPlaceHolderName + " AND rp" + cnt + + ".PRINCIPAL_TYPE='group' AND r" + cnt + + ".FROM_PRINCIPAL_ID=SECURITY_PRINCIPAL.PRINCIPAL_ID"; + } else { + groupConstraints = " AND r" + cnt + ".ASSOC_NAME='" + JetspeedPrincipalAssociationType.IS_MEMBER_OF + + "' AND r" + cnt + ".TO_PRINCIPAL_ID=rp" + cnt + ".PRINCIPAL_ID AND rp" + cnt + + ".PRINCIPAL_NAME LIKE " + _paramPlaceHolderName + " AND rp" + cnt + + ".PRINCIPAL_TYPE='group' AND r" + cnt + + ".FROM_PRINCIPAL_ID=SECURITY_PRINCIPAL.PRINCIPAL_ID"; + } + } + + fromPart += ", SECURITY_PRINCIPAL_ASSOC r" + cnt + ", SECURITY_PRINCIPAL rp" + cnt; + cnt++; + } + + // find principals that contain one or many users + // the principal must contain all supplied users. + String userConstraints = null; + + if (queryContext.getAssociatedUsers() != null && queryContext.getAssociatedUsers().size() > 0) { + int cnt = 1; + + for (String userName : queryContext.getAssociatedGroups()) { + _paramPlaceHolderName = putParamPlaceHolder(_paramPlaceHolders, userName); + + if (userConstraints == null) { + userConstraints = "r" + cnt + ".ASSOC_NAME='" + JetspeedPrincipalAssociationType.IS_MEMBER_OF + + "' AND r" + cnt + ".FROM_PRINCIPAL_ID=rp" + cnt + ".PRINCIPAL_ID AND rp" + cnt + + ".PRINCIPAL_NAME LIKE " + _paramPlaceHolderName + " AND rp" + cnt + + ".PRINCIPAL_TYPE='user' AND r" + cnt + ".TO_PRINCIPAL_ID=SECURITY_PRINCIPAL.PRINCIPAL_ID"; + } else { + userConstraints = " AND r" + cnt + ".ASSOC_NAME='" + JetspeedPrincipalAssociationType.IS_MEMBER_OF + + "' AND r" + cnt + ".FROM_PRINCIPAL_ID=rp" + cnt + ".PRINCIPAL_ID AND rp" + cnt + + ".PRINCIPAL_NAME LIKE " + _paramPlaceHolderName + " AND rp" + cnt + + ".PRINCIPAL_TYPE='group' AND r" + cnt + + ".TO_PRINCIPAL_ID=SECURITY_PRINCIPAL.PRINCIPAL_ID"; + } + } + + fromPart += ", SECURITY_PRINCIPAL_ASSOC r" + cnt + ", SECURITY_PRINCIPAL rp" + cnt; + cnt++; + } + + if (attributeConstraint != null) { + if (constraint != null) { + constraint += " AND " + attributeConstraint; + } else { + constraint = attributeConstraint; + } + } + + if (roleConstraints != null) { + if (constraint != null) { + constraint += " AND " + roleConstraints; + } else { + constraint = roleConstraints; + } + } + + if (groupConstraints != null) { + if (constraint != null) { + constraint += " AND " + groupConstraints; + } else { + constraint = groupConstraints; + } + } + + if (userConstraints != null) { + if (constraint != null) { + constraint += " AND " + userConstraints; + } else { + constraint = userConstraints; + } + } + + String baseSqlStr = "SELECT SECURITY_PRINCIPAL.* from " + fromPart + + " WHERE SECURITY_PRINCIPAL.PRINCIPAL_TYPE='" + queryContext.getJetspeedPrincipalType() + + "' AND SECURITY_PRINCIPAL.DOMAIN_ID=" + queryContext.getSecurityDomain(); + + if (constraint != null) { + baseSqlStr += " AND " + constraint; + } + + if (queryContext.getOrder() != null && queryContext.getOrder().equalsIgnoreCase("desc")) { + baseSqlStr += " ORDER BY SECURITY_PRINCIPAL.PRINCIPAL_NAME DESC"; + } else { + baseSqlStr += " ORDER BY SECURITY_PRINCIPAL.PRINCIPAL_NAME"; + } + + StringBuilder preparedSqlBuilder = new StringBuilder(baseSqlStr.length()); + Matcher matcher; + char [] sqlChars = baseSqlStr.toCharArray(); + List parameters = new ArrayList(_paramPlaceHolders.size()); + Segment segment = new Segment(sqlChars, 0, sqlChars.length); + + for (matcher = PARAM_PLACEHOLDER_PATTERN.matcher(segment); matcher.find(); ) { + preparedSqlBuilder.append(segment.subSequence(0, matcher.start())).append('?'); + _paramPlaceHolderName = matcher.group(1); + parameters.add(_paramPlaceHolders.get(_paramPlaceHolderName)); + segment = (Segment) segment.subSequence(matcher.end(), segment.length()); + matcher.reset(segment); + } + + preparedSqlBuilder.append(segment); + + String preparedSqlStr = preparedSqlBuilder.toString(); + String sql = getPagingSql(preparedSqlStr, queryContext); + + PreparedStatement pstmtForPaging = conn.prepareStatement(sql, ResultSet.TYPE_SCROLL_INSENSITIVE, + ResultSet.CONCUR_READ_ONLY); + for (int i = 0; i < parameters.size(); i++) { + pstmtForPaging.setString(i + 1, (String) parameters.get(i)); + } + pstmtForPaging.setFetchSize((int) (queryContext.getOffset() + queryContext.getLength())); + + String countSql = convertToCountQueryStatement(preparedSqlStr); + + PreparedStatement pstmtForCount = conn.prepareStatement(countSql); + for (int i = 0; i < parameters.size(); i++) { + pstmtForCount.setString(i + 1, (String) parameters.get(i)); + } + + return new PreparedStatement[] { pstmtForPaging, pstmtForCount }; + } + + private String convertToCountQueryStatement(String baseSqlStr) { + // get the total number of results effected by the query + int fromPos = baseSqlStr.toUpperCase().indexOf(" FROM "); + String countSql = "SELECT count(SECURITY_PRINCIPAL.PRINCIPAL_ID)" + baseSqlStr.substring(fromPos); + + // strip ORDER BY clause + int orderByPos = countSql.toUpperCase().indexOf(" ORDER BY "); + + if (orderByPos >= 0) { + countSql = countSql.substring(0, orderByPos); + } + + return countSql; + } + + /** * Generate the base SQL syntax for selecting principals. This must not * contain any database specifics. * * @param queryContext * @return + * @deprecated Never use this method due to vulnerable SQL statement manipulation. */ + @Deprecated protected String generateBaseSql(JetspeedPrincipalQueryContext queryContext) { String attributeConstraint = null; String fromPart = "SECURITY_PRINCIPAL"; --------------------------------------------------------------------- To unsubscribe, e-mail: jetspeed-dev-unsubscribe@portals.apache.org For additional commands, e-mail: jetspeed-dev-help@portals.apache.org