Security: Possible SQL Injection Fixed

- Instructional Offerings: Search, Worksheet PDF - Instructors: Manage Instructors List - Buildings: Update Rooms
realkjw · Jun 10, 2016 · f06f8b5 · f06f8b5
1 parent 5374ed5
commit f06f8b5
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 44 deletions.
diff --git a/JavaSource/org/unitime/timetable/model/InstructionalOffering.java b/JavaSource/org/unitime/timetable/model/InstructionalOffering.java
@@ -233,26 +233,24 @@ public static TreeSet<InstructionalOffering> search(
 		query.append(" where io.session.uniqueId=:sessionId ");
 
 		if (courseNbr != null && courseNbr.length() > 0){
-		    query.append(" and co.courseNbr ");
-		    if (courseNbr.indexOf('*')>=0) {
-	            query.append(" like '");
-	            courseNbr = courseNbr.replace('*', '%');
-		    }
-		    else {
-	            query.append(" = '");
-		    }
-            if (ApplicationProperty.CourseOfferingNumberUpperCase.isTrue())
-            	courseNbr = courseNbr.toUpperCase();
-            query.append(courseNbr);
-            query.append("'  ");
-        }
+			if (courseNbr.indexOf('*') >= 0) {
+				query.append(" and co.courseNbr like :courseNbr ");
+			} else {
+				query.append(" and co.courseNbr = :courseNbr ");
+			}
+		}
 
 		query.append(" and co.subjectArea.uniqueId = :subjectAreaId ");
 
 		Query q = hibSession.createQuery(query.toString());
 		q.setFetchSize(1000);
 		q.setLong("subjectAreaId", subjectAreaId);
 		q.setLong("sessionId", acadSessionId.longValue());
+		if (courseNbr != null && courseNbr.length() > 0) {
+			if (ApplicationProperty.CourseOfferingNumberUpperCase.isTrue())
+            	courseNbr = courseNbr.toUpperCase();
+			q.setString("courseNbr", courseNbr.replace('*', '%'));
+		}
 		q.setCacheable(true);
 
 

diff --git a/JavaSource/org/unitime/timetable/model/Room.java b/JavaSource/org/unitime/timetable/model/Room.java
@@ -27,7 +27,9 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.hibernate.Query;
 import org.hibernate.criterion.Restrictions;
+import org.hibernate.type.StringType;
 import org.unitime.timetable.defaults.ApplicationProperty;
 import org.unitime.timetable.model.base.BaseRoom;
 import org.unitime.timetable.model.dao.ExternalRoomDAO;
@@ -161,18 +163,15 @@ public static void addNewExternalRoomsToSession(Session session) {
 		boolean resetRoomFeatures = ApplicationProperty.BuildingsExternalUpdateExistingRoomFeatures.isTrue();
 		boolean resetRoomDepartments = ApplicationProperty.BuildingsExternalUpdateExistingRoomDepartments.isTrue();
 		String classifications = ApplicationProperty.BuildingsExternalUpdateClassification.value();
-		if (classifications != null) {
-			String classificationsQuery = "";
-			for (String c: classifications.split(",")) {
-				if (c.trim().isEmpty()) continue;
-				if (!classificationsQuery.isEmpty()) classificationsQuery += ", ";
-				classificationsQuery += "'" + c.trim() + "'";
-			}
-			if (!classificationsQuery.isEmpty())
-				query += " and er.classification in (" + classificationsQuery + ")";
+		if (classifications != null && !classifications.isEmpty()) {
+			query += " and er.classification in :classifications";
 		}
 		org.hibernate.Session hibSession = ExternalRoomDAO.getInstance().getSession();
-		for (ExternalRoom er: (List<ExternalRoom>)hibSession.createQuery(query).setLong("sessionId", session.getUniqueId()).list()) {
+		Query q = hibSession.createQuery(query).setLong("sessionId", session.getUniqueId());
+		if (classifications != null && !classifications.isEmpty()) {
+			q.setParameterList("classifications", classifications.split(","), new StringType());
+		}
+		for (ExternalRoom er: (List<ExternalRoom>)q.list()) {
 			Building b = Building.findByExternalIdAndSession(er.getBuilding().getExternalUniqueId(), session);
 			if (b == null) {
 				b = new Building();

diff --git a/JavaSource/org/unitime/timetable/model/Staff.java b/JavaSource/org/unitime/timetable/model/Staff.java
@@ -65,17 +65,12 @@ public static List getStaffByDept(String deptCode, Long acadSessionId) throws Ex
 			return(null);
 		}
 
-		StaffDAO sdao = new StaffDAO();
-		String sql = "select distinct s " +
-					 "from Staff s " +
-					 "where s.dept='" + deptCode + "'" +
-					 "  and ( " +
-						 "select di.externalUniqueId " +
-						 "from DepartmentalInstructor di " +
-						 "where di.department.deptCode='" + deptCode + "' " + 
-						 "  and di.department.session.uniqueId=" + acadSessionId.toString() +
-						 "  and di.externalUniqueId = s.externalUniqueId ) is null";
-		Query q = sdao.getSession().createQuery(sql);
+		Query q = StaffDAO.getInstance().getSession().createQuery(
+				"select distinct s from Staff s where s.dept=:deptCode and " +
+				"(select di.externalUniqueId from DepartmentalInstructor di " +
+				"where di.department.deptCode=:deptCode and di.department.session.uniqueId=:sessionId and di.externalUniqueId = s.externalUniqueId ) is null");
+		q.setString("deptCode", deptCode);
+		q.setLong("sessionId", acadSessionId);
 		q.setCacheable(true);
 		return (q.list());
 	}

diff --git a/JavaSource/org/unitime/timetable/util/PdfWorksheet.java b/JavaSource/org/unitime/timetable/util/PdfWorksheet.java
@@ -38,6 +38,7 @@
 
 import org.cpsolver.coursett.model.RoomLocation;
 import org.cpsolver.coursett.model.TimeLocation;
+import org.hibernate.Query;
 import org.unitime.commons.hibernate.util.HibernateUtil;
 import org.unitime.localization.impl.Localization;
 import org.unitime.timetable.ApplicationProperties;
@@ -168,18 +169,22 @@ public int compare(Object o1, Object o2) {
                 return co1.getUniqueId().compareTo(co2.getUniqueId());
             }
         });
-        String subjectIds = "";
+        List<Long> subjectIds = new ArrayList<Long>();
         for (SubjectArea sa: subjectAreas)
-        	subjectIds += (subjectIds.isEmpty() ? "" : ",") + sa.getUniqueId();
-        String query = "select co from CourseOffering co where  co.subjectArea.uniqueId in (" + subjectIds + ")";
-        if (courseNumber!=null && !courseNumber.trim().isEmpty()) {
-            query += " and co.courseNbr ";
-            if (courseNumber.indexOf('*')>=0)
-                query += " like '"+courseNumber.trim().replace('*', '%').toUpperCase()+"'";
-            else 
-                query += " = '"+courseNumber.trim().toUpperCase()+"'";
+        	subjectIds.add(sa.getUniqueId());
+        String query = "select co from CourseOffering co where  co.subjectArea.uniqueId in :subjectIds";
+        if (courseNumber != null && !courseNumber.trim().isEmpty()) {
+			if (courseNumber.indexOf('*') >= 0) {
+				query += " and co.courseNbr like :courseNbr ";
+			} else {
+				query += " and co.courseNbr = :courseNbr ";
+			}
         }
-        courses.addAll(new SessionDAO().getSession().createQuery(query).list());
+        Query q = new SessionDAO().getSession().createQuery(query);
+        q.setParameterList("subjectIds", subjectIds);
+        if (courseNumber != null && !courseNumber.trim().isEmpty())
+        	q.setParameter("courseNbr", ApplicationProperty.CourseOfferingNumberUpperCase.isTrue()? courseNumber.trim().replace('*', '%').toUpperCase() : courseNumber.trim().replace('*', '%'));
+        courses.addAll(q.list());
         if (courses.isEmpty()) return false;
         PdfWorksheet w = new PdfWorksheet(out, subjectAreas, courseNumber);
         for (Iterator i=courses.iterator();i.hasNext();) {

diff --git a/WebContent/help/Release-Notes.xml b/WebContent/help/Release-Notes.xml
@@ -27,6 +27,24 @@
 <release>
 	<version>4.1.${build.number}</version>
 	<release-date>${build.date}</release-date>
+	<category>
+		<title>Other</title>
+		<item>
+			<name>Security: Possible SQL Injection Fixed</name>
+			<description>
+				<line>Following features were corrected:
+					<line>Instructional Offerings: Search, Worksheet PDF (course number)</line>
+					<line>Instructors: Manage Instructors List (department code)</line>
+					<line>Buildings: Update Rooms (room classifications)</line>
+				</line>
+			</description>
+		</item>
+	</category>
+</release>
+
+<release>
+	<version>4.1.186</version>
+	<release-date>Sat, 4 Jun 2016</release-date>
 	<category>
 		<title>Event Management</title>
 		<item>