Menu

#1451 Nonconstant string passed to execute method on an SQL statement incorrectly marked

3.0.1
closed-rejected
nobody
sql (1)
5
2017-10-22
2016-04-27
No

Findbugs incorrectly marked the following code as:

Bug: at.gecf.dolphin.app.vertrag.dao.ContractServiceDAO.getLeaseAgreementDetails(ContractListFilter) passes a nonconstant String to an execute or addBatch method on an SQL statement

The method invokes the execute or addBatch method on an SQL statement with a String that seems to be dynamically generated. Consider using a prepared statement instead. It is more efficient and less vulnerable to SQL injection attacks.

--> rs = stmt.executeQuery(query);

At least one of the conditions is true and does not generate an invalid query

 public List<ContractListItem> getLeaseAgreementDetails(ContractListFilter filter) throws DBException, ApplicationException {

            //validateContractListFilter(filter);


            if (filter.validateRules().isEmpty()) {
                    if(filter.getKdnr() == 0 
                                    && StringUtils.isBlank(filter.getVtnr())
                                    && filter.getRolle() == null
                                    && filter.getFrom() == null
                                    && filter.getUntil() == null) {

                            throw new ApplicationException("Query must be restricted with at least one ContractListFilter value.");
                    }

            } else {
                    throw new ApplicationException("One or more ContractListFilter values are invalid.");
            }

            List<ContractListItem> list = new ArrayList<ContractListItem>();
            Connection con = null;
            Statement stmt = null;
            ResultSet rs = null;

            try {
                ArrayList<KeyValuePair> redAgrTypeCache = getApplicationCache() == null 
                        ? new ArrayList<KeyValuePair>() : getApplicationCache().getVarianteRedAgrType();
                        con = getConnection();
                        String query = LEASE_AGREEMENT_DETAILS_QUERY + WHERE;

                        boolean preceeded = false;
                        if(filter.getKdnr() > 0) {

                            query = query + KDNR_EQUALS + filter.getKdnr();
                            preceeded = true;
                        }
                        if(!StringUtils.isBlank(filter.getVtnr())) {

                            query = preceeded ? query  + AND + VTNR_EQUALS : query + VTNR_EQUALS;
                            query = query + filter.getVtnr();
                            preceeded = true;
                        }
                        if(filter.getRolle() != null) {

                            query = preceeded ? query  + AND + ROLLE_EQUALS : query + ROLLE_EQUALS;
                            query = query + "'" + filter.getRolle().getValue() + "'";
                            preceeded = true;
                        }
                        if(filter.getFrom() != null) {
                            query = preceeded ? query  + AND + VERTRAGS_ENDE_AFTER : query + VERTRAGS_ENDE_AFTER;
                            query = query + "to_date('" + DolphinDate.formatToString(filter.getFrom()) + "', 'dd-MM-yyyy')";
                        }

                        stmt = con.createStatement();
                        rs = stmt.executeQuery(query);

                        while (rs.next()) {

                           //Etc.

                        }
            } catch (SQLException e) {
                //Exception handled
            } finally {
                //Releases resources
            }

            return list;
        }

Discussion

  • Marco Tulio Avila Cerón

    Also SQL injection attacks are not coming here, how does findbugs can asses that?

     
  • Andrey Loskutov

    Andrey Loskutov - 2017-10-22
    • Status: open --> closed-rejected
     

Log in to post a comment.

MongoDB Logo MongoDB