EntityManager create native query vs persist and injections

Question

Is createNativeQuery() safe against SQL injection if used as in:

@ManagedBean
@ViewScoped
public class UserController {

    @PersistenceContext
    private EntityManager em;

    public User register(User u) {
        Query query = em.createNativeQuery("SELECT r1_register(?,?,?,?,?,?,?)");
        short i = 0;
        query.setParameter(++i, u.getUsername());
        query.setParameter(++i, u.getPassword());
        query.setParameter(++i, u.getName());
        query.setParameter(++i, u.getSurname());
        query.setParameter(++i, u.getEmail());
        query.setParameter(++i, u.getBirthdate());
        query.setParameter(++i, u.getPhoneNumber());
        int id = (int) query.getSingleResult();
        if (id != 0) u.setIduser(id);
        return u;
    }
}

r1_register is a stored function that performs an INSERT and returns the id of the newly inserted user. Would this be equivalent:

public User register(User u) {
    em.persist(u);
    // get the last inserted id (user id must be @Generated)
    em.flush(); // user id set here
    return u;
}

u is in both cases filled by the user. Finally is a transaction initiated by default ?

EDIT: The routine:

CREATE DEFINER=`root`@`localhost` FUNCTION `r1_register`(username VARCHAR(45),
                _password VARCHAR(45),
                _name VARCHAR(45),
                surname VARCHAR(45),
                _email VARCHAR(45),
                _birthdate DATE,
                phone_number VARCHAR(10) ) RETURNS int(11)
BEGIN
-- Adds a new user.
    -- START TRANSACTION; -- Begin a transaction -- NOT ALLOWED
    -- http://stackoverflow.com/questions/16969875/
    IF r1_check_unique_username(username)=0 THEN
        RETURN 0;
    END IF;
    INSERT IGNORE INTO `hw1_db`.`users` (`username`, `password`, `name`, `surname`, `email`, `birthdate`, `phone_number`)
        VALUES (username, _password, _name, surname, _email, _birthdate, phone_number);
    -- see: http://stackoverflow.com/a/5939840/281545
    -- The drawback to this approach is that you cannot go back and use
    -- ids wasted because of failed attempts to INSERT IGNORE in the event
    -- of a duplicate key. Shouldn't be a problem for us as we check.
    -- /Transaction
    -- IF ROW_COUNT() > 0 THEN
    -- ROW_COUNT() returns the number of rows updated/inserted/deleted
    --  COMMIT; -- Finalize the transaction
    -- ELSE
    --  ROLLBACK; -- Revert all changes made before the transaction began
    -- END IF;
    RETURN LAST_INSERT_ID();
END

Generally, parameterized SQL constructs (e.g. `PreparedStatement`) escapes special characters by default and in so doing, you're protected against vanilla SQL injections — kolossus, Feb 08 '14 at 23:29
@kolossus: yes I am asking about the JPA api - does for instance `persist(Entity e)` internally escape String fields of the Entity it persists — Mr_and_Mrs_D, Feb 08 '14 at 23:32

Petr Mensik · Accepted Answer · 2014-02-08T18:36:07.360

1

This depends on what r1_register is actually doing. If it just saving the user and nothing else, than yes, they are equivalent because that's what EntityManager#persist is doing. But if the DB function is performing some security checking or writing to other tables, than you need to implement it in JPA too. Btw code for inserting User and obtaining the ID should be

public User register(User u) {
    em.getTransaction().begin();
    em.persist(u);
    em.getTransaction().commit();
    int id = u.getId();
    return u;
}

But you don't have to call EntityManager#flush if you need that id after the register method is called, flush is performed by the end of every transaction.

edited Feb 08 '14 at 18:36

answered Feb 08 '14 at 11:52

Petr Mensik

26,874
17
90
115

So they both guard against sql infection ? Also they both initiate a transaction ? Mycode is as seen. Also `e.getId();` ? There is no e variable – Mr_and_Mrs_D Feb 08 '14 at 14:42
Sorry, I meant `u`. Well, `EntityManager` certainly protects you from SQL injection. And no, transaction is iniatiated by entering to the method in `EJB` bean or through `em.getTransaction().begin();`. – Petr Mensik Feb 08 '14 at 15:05
You are wrong I am afraid `em.persist(u);` with or without flush _does not set the id_ - `System.out.println("USER id: " + u.getIduser());` prints 0. I still wonder if injection is prevented - I am surprised they don't mention it in the docs. Also I am not still sure there is transaction going on - could you provide some links ? – Mr_and_Mrs_D Feb 08 '14 at 18:07
EM wouldn't even insert your entity into DB if there is no active transaction. It depends in which context you are calling it - is method `register` part of EJB bean?If yes, you've got transaction out of the box, if not, than you need to begin and commit it by yourself. – Petr Mensik Feb 08 '14 at 18:12
And for SQLi, you are really safe with EM unless you are concatenating variables to a SQL quries. – Petr Mensik Feb 08 '14 at 18:14
So is `persist()` internally escaping String fields of the Entity ? I 'd rather have some reference on this. – Mr_and_Mrs_D Feb 08 '14 at 18:16
I've edited my answer to show you how to use transactions. And for the reference, I am sorry but I can't find anything but I am absolutely sure that EM escapes it - it would be quite useless if the JPA wouldn't protect from SQLi. – Petr Mensik Feb 08 '14 at 18:40
Edited my question to display the routine - would your `em.getTransaction().begin();...em.getTransaction().commit();` work the same with the `createNativeQuery` way ? – Mr_and_Mrs_D Feb 08 '14 at 18:51
+1 for getting me to the right track with the id - it had to be annotated with @Generated to (`em.persist()` would work but I needed flush to get the id written in the entity - weird). Last thing I need you to tell me. Could your answer be modified so the transaction is declarative (ie I do not `begin` and `commit` manually) ? – Mr_and_Mrs_D Feb 09 '14 at 17:10
You would have to use EJB bean or switch to CDI 1.1 to get declarative transactions, you unfortunately won't achieve this in JSF bean. – Petr Mensik Feb 09 '14 at 21:00
So `register()` is not executed inside a transaction ? – Mr_and_Mrs_D Feb 09 '14 at 21:19
No, like I said you need either EJB or newest CDI in order to get declarative transactions. – Petr Mensik Feb 09 '14 at 21:25

EntityManager create native query vs persist and injections

1 Answers1

Linked