JPA -> Dynamische WHERE Clause / SQL Injection möglich?

[beta20]

Hallo,

ich halte es derzeit so, dass ich von meiner CDI Bean "Such-Objekte" in meinen EJB Container weitergebe, die die Parameter und deren Value für meine WHERE Clause Abfrage enthält. Ich möchte nicht für jede einzelne Query, die ich irgendwann mal benötige eine eigene Funktion erstellen....
-> Zunächst muss ich sagen, dass das funktioniert und meine Queries auch immer das richtige Ergebnis zurückgeben.

Allerdings bin ich mir nicht sicher, ob ich hierdurch mir eventuell Sicherheitslücken baue? Ggf. buse ich auch etwas an Performance ein?
Wie es mir scheint, wäre hierdurch dann SQL Injection möglich?

Hierzu habe ich dann eine Klasse "ObjectForSearchList":

public class ObjectForSearchList {
    private String searchName;
    private String searchValue;
    private Date searchValueDate;
    private List<String> searchValueStringList;

    private double searchNameNumber;
    private double searchValueNumber;

    private String dataType;

    public ObjectForSearchList(String searchName, String searchValue) {
        super();
        this.searchName = searchName;
        this.searchValue = searchValue;
    }

    public ObjectForSearchList() {
        super();
    }

    public ObjectForSearchList(String searchName, double searchValueNumber) {
        super();
        this.searchName = searchName;
        this.searchValueNumber = searchValueNumber;
    }

    public ObjectForSearchList(String searchName, String searchValue, String dataType) {
        super();
        this.searchName = searchName;
        this.searchValue = searchValue;
        this.dataType = dataType;
    }

    public ObjectForSearchList(double searchNameNumber, double searchValueNumber) {
        super();
        this.searchNameNumber = searchNameNumber;
        this.searchValueNumber = searchValueNumber;
    }

    public Date getSearchValueDate() {
        return searchValueDate;
    }

    public void setSearchValueDate(Date searchValueDate) {
        this.searchValueDate = searchValueDate;
    }

    public double getSearchNameNumber() {
        return searchNameNumber;
    }

    public void setSearchNameNumber(double searchNameNumber) {
        this.searchNameNumber = searchNameNumber;
    }

    public String getSearchName() {
        return searchName;
    }

    public void setSearchName(String searchName) {
        this.searchName = searchName;
    }

    public String getSearchValue() {
        return searchValue;
    }

    public void setSearchValue(String searchValue) {
        this.searchValue = searchValue;
    }

    public String getDataType() {
        return dataType;
    }

    public void setDataType(String dataType) {
        this.dataType = dataType;
    }

    public double getSearchValueNumber() {
        return searchValueNumber;
    }

    public void setSearchValueNumber(double searchValueNumber) {
        this.searchValueNumber = searchValueNumber;
    }

    public List<String> getSearchValueStringList() {
        return searchValueStringList;
    }

    public void setSearchValueStringList(List<String> searchValueStringList) {
        this.searchValueStringList = searchValueStringList;
    }

    @Override
    public String toString() {
        return "ObjectForSearchList [searchName=" + searchName + ", searchValue=" + searchValue + ", searchValueDate="
                + searchValueDate + ", searchValueStringList=" + searchValueStringList + ", searchNameNumber="
                + searchNameNumber + ", searchValueNumber=" + searchValueNumber + ", dataType=" + dataType + "]";
    }

    @Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + ((dataType == null) ? 0 : dataType.hashCode());
        result = prime * result + ((searchName == null) ? 0 : searchName.hashCode());
        long temp;
        temp = Double.doubleToLongBits(searchNameNumber);
        result = prime * result + (int) (temp ^ (temp >>> 32));
        result = prime * result + ((searchValue == null) ? 0 : searchValue.hashCode());
        result = prime * result + ((searchValueDate == null) ? 0 : searchValueDate.hashCode());
        temp = Double.doubleToLongBits(searchValueNumber);
        result = prime * result + (int) (temp ^ (temp >>> 32));
        result = prime * result + ((searchValueStringList == null) ? 0 : searchValueStringList.hashCode());
        return result;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        ObjectForSearchList other = (ObjectForSearchList) obj;
        if (dataType == null) {
            if (other.dataType != null)
                return false;
        } else if (!dataType.equals(other.dataType))
            return false;
        if (searchName == null) {
            if (other.searchName != null)
                return false;
        } else if (!searchName.equals(other.searchName))
            return false;
        if (Double.doubleToLongBits(searchNameNumber) != Double.doubleToLongBits(other.searchNameNumber))
            return false;
        if (searchValue == null) {
            if (other.searchValue != null)
                return false;
        } else if (!searchValue.equals(other.searchValue))
            return false;
        if (searchValueDate == null) {
            if (other.searchValueDate != null)
                return false;
        } else if (!searchValueDate.equals(other.searchValueDate))
            return false;
        if (Double.doubleToLongBits(searchValueNumber) != Double.doubleToLongBits(other.searchValueNumber))
            return false;
        if (searchValueStringList == null) {
            if (other.searchValueStringList != null)
                return false;
        } else if (!searchValueStringList.equals(other.searchValueStringList))
            return false;
        return true;
    }
}

Beispiel:

CDI Bean:

List<ObjectForSearchList> searchList = new ArrayList<ObjectForSearchList>();
searchList = new ArrayList<ObjectForSearchList>();
searchList.add(new ObjectForSearchList("idHash", chatUserId));
chatUser = chatUserService.findChatUserByQuery(searchList);

EJB
Hier habe ich eine Methode, die mir die WHERE Clause zusammenbastelt:

/**
     * Query erstellen
     *
     * @param searchList
     * @return
     */
    private StringBuilder createQuery(List<ObjectForSearchList> searchList) {

        StringBuilder where = new StringBuilder();

        // Search
        int andValues = 0;
        for (ObjectForSearchList o : searchList) {

            if (andValues == 0)
                where.append(" WHERE ");

            if (o.getSearchName().equals("idHash") && o.getSearchValue() != null) {
                if (andValues != 0)
                    where.append(" AND ");

                where.append(" m.idHash = '" + o.getSearchValue() + "'");
                andValues++;
            }
            // Where löschen, wenn nichts in WHERE - Clause
            if (andValues == 0) {
                where = new StringBuilder();
            }
        }
        return where;
}

Und die Methode zum Suchen sieht eben so aus:
-> Hier ist dann der Aufruf von createQuery(searchList), welches mir die WHERE Clause als String zurückgibt...

/**
     * Filtern
     */
    public ChatUser findChatUserByQuery(List<ObjectForSearchList> searchList) {

        LOGGER.debug("START findChatUserByQuery");

        ChatUser chatUser = null;

        try {
            StringBuilder queryCount = new StringBuilder("SELECT m FROM ChatUser m");
            StringBuilder where = new StringBuilder();
            StringBuilder orderBy = new StringBuilder(" ORDER BY m.id ASC");

            String theQuery = "";
            theQuery = queryCount.toString();

            // WHERE Clause
            where = createQuery(searchList);

            queryCount.append((where == null ? "" : where));
            queryCount.append((orderBy == null ? "" : orderBy));

            theQuery = queryCount.toString();
            LOGGER.debug("QUERY: " + theQuery);

            Query q = entityManager.createQuery(theQuery);

            chatUser = (ChatUser) q.getSingleResult();

            if (chatUser == null)
                return null;

        } catch (Exception e) {
            return null;
        }

        LOGGER.debug("Find ChatUser with ID: " + chatUser.getId());
        LOGGER.debug("END findChatUserByQuery");
        return chatUser;
    }

Wie könnte ich das anderst lösen? Primär geht es mir um Sicherheit. Funktionieren tut es ja...
Zudem suche ich hierüber nicht immer nur Attribute ab, sondern ich habe zB auch sowas:

searchList.add(new ObjectForSearchList("findMyUsers", 1));
-> Hiermit baue ich ich dann in der createQuery() mir ein SubSelect user.id = (Select.... FROM UserClass u WHERE i.id = o.getSearchValue())

Leider habe ich mein ganzes Projekt mittlerweile so aufgebaut - und das über 100 Klassen, die ich dann anpassen müsste...
Danke für die Hilfe...

 

[thecain]

Warum nicht einfach NamedParameters oder den QueryBuilder verwenden, statt so ein eigenes Konstrukt? mMn auch anfällig gegen Injection...

 

[beta20]

Hast du ein Beispiel dafür?

 

[thecain]

Von was?

 

[beta20]

Wie man solch eine Query dann baut, analog wie ich es oben mache?

 

[LimDul]

Z.b. Hier:
https://www.baeldung.com/jpa-query-parameters

Anstelle den Wert direkt zu setzen, einen Named Parameter definieren und den dann setzen.

 

[beta20]

hm, also eigentlich würde ich gerne das Konstrukt beibehalten...

Im Prinzip baue ich mir hiermit meine WHERE - Clause zusammen. Am Ende kommt ein String zurück, den ich dann der Query selbst hinzufüge.

private StringBuilder createQuery(List<ObjectForSearchList> searchList) {

        StringBuilder where = new StringBuilder();
       
 // Search
        int andValues = 0;
        for (ObjectForSearchList o : searchList) {

            if (andValues == 0)
                where.append(" WHERE ");

            if (o.getSearchName().equals("idHash") && o.getSearchValue() != null) {
                if (andValues != 0)
                    where.append(" AND ");

                where.append(" m.idHash = '" + o.getSearchValue() + "'");
                andValues++;
            }

StringBuilder queryCount = new StringBuilder("SELECT m FROM EmailTemplate m");
StringBuilder where = new StringBuilder();

            String theQuery = "";
            theQuery = queryCount.toString();

            // WHERE Clause
            where = createQuery(searchList);

            theQuery = queryCount.toString();

            Query q = entityManager.createQuery(theQuery);
            emailTemplate = (EmailTemplate) q.getSingleResult();

Wie könnte ich das Konstrukt denn weiterhin benutzen, aber dann eben mit NamedQueries o.ä. arbeiten?
Also wenn ich das richtig sehe, dann fülle ich eben nicht die WHERE Clause mit den richtigen Werten, sondern schreibe anhand dem Wert einen Parameter rein:
SELECT e FROM EmailTemplate e WHERE e.name = :name

Das heißt von:
private StringBuilder createQuery(List<ObjectForSearchList> searchList)

bekomme ich immer noch einen String zurück, der mir die WHERE Clause generiert.
Zusätzlich aber auch noch eine Liste von Parameter.
Also würde ich bspw. eine neue Klasse benötigen:

JpaHelperClass:
Attribute:
String where;
List<Parameter> parameterList;

Wenn ich das Beispiel von dem Link nehme, benötige ich dann sowas:

JpaHelperClass whereClause = createQuery(List<ObjectForSearchList> searchList)
String w = where.getWhere();
TypedQuery<Employee> query = em.createQuery("SELECT e FROM Employee e " + w , Employee.class);

-> Nun die Parameter und Werte hinzufügen von JpaHelperClass.getParameterList.

Wie mache ich das hier aber programmiertechnnisch? Wie füge ich hier diese setParameter dem query Objekt hinzu?
List<Employee> employees = query
.setParameter("name", empName)
.setParameter("empAge", empAge)
.getResultList();

 

[mihe7]

Wie mache ich das hier aber programmiertechnnisch? Wie füge ich hier diese setParameter dem query Objekt hinzu?

Verstehe das Problem nicht. Das funktioniert doch nicht anders, wie das Zusammenbauen des Strings.

 

[beta20]

hm... sowas wie
List<ObjectForSearchList> searchList;

for(ObjectForSearchList o : searchList){
query..setParameter(o.getName(), o.getValue())
    }

 

[mihe7]

Zum Bleistift. Du wirst aber noch die null-Prüfung brauchen, weil Du den Parameter ja nur dann hinzufügst, wenn der Wert != null ist.

 

[beta20]

Ich habe hierzu nochmal eine Frage:
Ich habe meine WHERE Clause immer so aufgebaut:

where.append(" m.idHash = '" + o.getSearchValue() + "'");

Hier habe ich dann eben auch berücksichtigt, ob mein Wert ein String oder eine Zahl ist.
Wenn es ein String war, habe ich eben die einfachen Anführungszeichen genommen:
m.idHash = ' <meinWert>'

bei einer Zahl dann eben nur so:
m.idHash = <meinWert>

Muss ich das dann ebenfalls berücksichtigen?
Was ich nun gemacht habe, ist mir eine Klasse zu schreiben:

public class JpaQueryBuilder {

    private String sqlString;
    private String parameterName;
    private String parameterValue;

sqlString ist dann der String mit der WHERE Clause, was ich zuvor aus StringBuilder where = new StringBuilder(); bekommen habe.
Anstatt den richtigen Wert in der SQL Abfrage dann zu speichern, füge ich eben den Parameter ein:

StringBuilder where = new StringBuilder();
JpaQueryBuilder queryBuilder = new JpaQueryBuilder();
...

if (o.getSearchName().equals(Constants.QUERY_ID_HASH) && o.getSearchValue() != null) {

                if (andValues != 0)
                    where.append(" AND ");


                where.append(" m.idHash = :" + Constants.QUERY_ID_HASH);   
                queryBuilder.setParameterName(Constants.QUERY_ID_HASH);
                queryBuilder.setParameterValue(o.getSearchValue());

Die Frage ist nun, ob ich bei dieser Zeile zwischen dem Datentyp unterscheiden muss:
where.append(" m.idHash = :" + Constants.QUERY_ID_HASH);

Also sprich bei Strings:
where.append(" m.idHash = ':" + Constants.QUERY_ID_HASH + "'");

und bei Zahlen
where.append(" m.idHash = :" + Constants.QUERY_ID_HASH);

später füge ich dann eben die Query zusammen:

    public Query createQuery(Query q, List<JpaQueryBuilder> parameterList) {

        if (parameterList == null || parameterList.isEmpty())
            return q;

        for (JpaQueryBuilder jpaQueryBuilder : parameterList) {
            q.setParameter(jpaQueryBuilder.getParameterName(), jpaQueryBuilder.getParameterValue());
        }

        return q;
    }

 

[mihe7]

Die Frage ist nun, ob ich bei dieser Zeile zwischen dem Datentyp unterscheiden muss:

Nein. Ein ':XY' wäre kein Parameter sondern ein String. Immer einfach den Parameter :XY hinzufügen. JPA weiß selbst, dass bei einem String Hochkommas zu verwenden sind.