ResetMsgSeqNum flag in Logon message is processed prior to application level logon message validation

[dilanperera87]

Describe the bug We’ve developed a fix trading gateway using quickfixj library and user validation is performed in fromAdmin callback function when receiving a logon message. If an user sends logon request with invalid credentials and ResetSeqNumFlag set to Y, then sequence numbers are getting reset prior to the application level logon message validation since fromAdmin callback is fired after resetting the session message store. Due to this issue, an unauthorized user will be able to reset session sequence numbers and messages.

To Reproduce

1. Establish TCP/IP connection and send Logon message (MsgSeqNum=1)
2. Acceptor accepts logon and send Logon response (MsgSeqNum=1)
3. Exchange few messages to increase sequence numbers.
4. Logout/disconnect session.
5. Assume next target sequence number is 5 and next sender sequence number is 10.
6. Re-establish TCP/IP connection and send Logon message with ResetSeqNumFlag=Y and MsgSeqNum=1. (Consider this as an invalid logon attempt)
7. Acceptor rejects logon request (fromAdmin function implementation throws RejectLogon exception) and connection gets terminated.
8. Re-establish TCP/IP connection and send Logon message (MsgSeqNum=5) to continue previous valid session disconnected at step 4.
9. Acceptor accepts logon and send Logon response with (MsgSeqNum=1)
10. Acceptor sends ResendRequest requesting messages from sequence number 2

Expected behavior MsgSeqNum should be 10 in the Logon response sent in step 9 above and acceptor should not request missed messages via ResendRequest as in step 10.

system information:

• OS: Linux
• Java version: JDK11
• QFJ Version: 2.3.0

[chrjohn]

Hmm, probably the verify() method call needs to be moved further up in nextLogon() but this needs to be tested carefully. Are you able to provide a unit test?

[dilanperera87]

Sure, I'll do the changes and test it locally first and the raise a PR.

[harinda05]

@chrjohn raised a PR for this issue. Please let me know if everything's in line. This is my first PR for QuickFixJ