Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 29c5bbf6 authored by Casper Bonde's avatar Casper Bonde Committed by Andre Eisenbach
Browse files

SAP: Fix logic to skip padding bytes for requests received



As per SAP spec, padding bytes can be 0-3 bytes, but the current code
has incorrect logic, that would lead to calculation of padding bytes
as 4 in case the APDU length was multiple of 4, which would lead to
incorrect parsing logic in SAP Server and can lead to issues of no
response for APDU request from SAP Server as it keeps waiting for
reading more bytes from rfcomm which are not present.

Also check added in code not to send msg to RIL if socket is null to
prevent crash in BT Sap module.

Bug: 23024598
Signed-off-by: default avatarCasper Bonde <c.bonde@samsung.com>
Change-Id: I24e4a6b850709c9c32b7e0992626a0219dacef03
parent fc42acc4
Loading
Loading
Loading
Loading
+19 −12
Original line number Diff line number Diff line
@@ -456,18 +456,25 @@ public class SapMessage {
        int paramId;
        int paramLength;
        boolean success = true;
        int skipLen = 0;

        for(int i = 0; i < count; i++) {
            paramId = is.read();
            is.read(); // Skip the reserved byte
            paramLength = is.read();
            paramLength = paramLength << 8 | is.read();

            // As per SAP spec padding should be 0-3 bytes
            if ((paramLength % 4) != 0)
                skipLen = 4 - (paramLength % 4);

            if(VERBOSE) Log.i(TAG, "parsing paramId: " + paramId + " with length: " + paramLength);
            switch(paramId) {
            case PARAM_MAX_MSG_SIZE_ID:
                if(paramLength != PARAM_MAX_MSG_SIZE_LENGTH) {
                    Log.e(TAG, "Received PARAM_MAX_MSG_SIZE with wrong length: " +
                            paramLength + " skipping this parameter.");
                    skip(is, paramLength + (4 - (paramLength % 4)));
                    skip(is, paramLength + skipLen);
                    success = false;
                } else {
                    mMaxMsgSize = is.read();
@@ -478,18 +485,18 @@ public class SapMessage {
            case PARAM_COMMAND_APDU_ID:
                mApdu = new byte[paramLength];
                read(is, mApdu);
                skip(is, 4 - (paramLength % 4));
                skip(is, skipLen);
                break;
            case PARAM_COMMAND_APDU7816_ID:
                mApdu7816 = new byte[paramLength];
                read(is, mApdu7816);
                skip(is, 4 - (paramLength % 4));
                skip(is, skipLen);
                break;
            case PARAM_TRANSPORT_PROTOCOL_ID:
                if(paramLength != PARAM_TRANSPORT_PROTOCOL_LENGTH) {
                    Log.e(TAG, "Received PARAM_TRANSPORT_PROTOCOL with wrong length: " +
                            paramLength + " skipping this parameter.");
                    skip(is, paramLength + (4 - (paramLength % 4)));
                    skip(is, paramLength + skipLen);
                    success = false;
                } else {
                    mTransportProtocol = is.read();
@@ -502,7 +509,7 @@ public class SapMessage {
                    if(paramLength != PARAM_CONNECTION_STATUS_LENGTH) {
                        Log.e(TAG, "Received PARAM_CONNECTION_STATUS with wrong length: " +
                                paramLength + " skipping this parameter.");
                        skip(is, paramLength + (4 - (paramLength % 4)));
                        skip(is, paramLength + skipLen);
                        success = false;
                    } else {
                        mConnectionStatus = is.read();
@@ -516,7 +523,7 @@ public class SapMessage {
                    if(paramLength != PARAM_CARD_READER_STATUS_LENGTH) {
                        Log.e(TAG, "Received PARAM_CARD_READER_STATUS with wrong length: " +
                                paramLength + " skipping this parameter.");
                        skip(is, paramLength + (4 - (paramLength % 4)));
                        skip(is, paramLength + skipLen);
                        success = false;
                    } else {
                        mCardReaderStatus = is.read();
@@ -530,7 +537,7 @@ public class SapMessage {
                    if(paramLength != PARAM_STATUS_CHANGE_LENGTH) {
                        Log.e(TAG, "Received PARAM_STATUS_CHANGE with wrong length: " +
                                paramLength + " skipping this parameter.");
                        skip(is, paramLength + (4 - (paramLength % 4)));
                        skip(is, paramLength + skipLen);
                        success = false;
                    } else {
                        mStatusChange = is.read();
@@ -544,7 +551,7 @@ public class SapMessage {
                    if(paramLength != PARAM_RESULT_CODE_LENGTH) {
                        Log.e(TAG, "Received PARAM_RESULT_CODE with wrong length: " +
                                paramLength + " skipping this parameter.");
                        skip(is, paramLength + (4 - (paramLength % 4)));
                        skip(is, paramLength + skipLen);
                        success = false;
                    } else {
                        mResultCode = is.read();
@@ -558,7 +565,7 @@ public class SapMessage {
                    if(paramLength != PARAM_DISCONNECT_TYPE_LENGTH) {
                        Log.e(TAG, "Received PARAM_DISCONNECT_TYPE_ID with wrong length: " +
                                paramLength + " skipping this parameter.");
                        skip(is, paramLength + (4 - (paramLength % 4)));
                        skip(is, paramLength + skipLen);
                        success = false;
                    } else {
                        mDisconnectionType = is.read();
@@ -571,7 +578,7 @@ public class SapMessage {
                if(TEST) {
                    mApduResp = new byte[paramLength];
                    read(is, mApduResp);
                    skip(is, 4 - (paramLength % 4));
                    skip(is, skipLen);
                    break;
                } // Fall through if TEST == false
            case PARAM_ATR_ID:
@@ -579,13 +586,13 @@ public class SapMessage {
                if(TEST) {
                    mAtr = new byte[paramLength];
                    read(is, mAtr);
                    skip(is, 4 - (paramLength % 4));
                    skip(is, skipLen);
                    break;
                } // Fall through if TEST == false
            default:
                Log.e(TAG, "Received unknown parameter ID: " + paramId + " length: " +
                        paramLength + " skipping this parameter.");
                skip(is, paramLength + (4 - (paramLength % 4)));
                skip(is, paramLength + skipLen);
            }
        }
        return success;
+2 −1
Original line number Diff line number Diff line
@@ -822,6 +822,7 @@ public class SapServer extends Thread implements Callback {
        if(VERBOSE) Log.i(TAG_HANDLER, "sendRilMessage() - "
                + SapMessage.getMsgTypeName(sapMsg.getMsgType()));
        try {
            if (mRilBtOutStream != null)
                sapMsg.writeReqToStream(mRilBtOutStream);
        } catch (IOException e) {
            Log.e(TAG_HANDLER, "Unable to send message to RIL", e);