OpenEtherCATsociety / SOES

Simple Open Source EtherCAT Slave
Other
588 stars 251 forks source link

PDO mapping overflow #95

Open EvgenyDD opened 3 years ago

EvgenyDD commented 3 years ago

There is a problem with MAX_RXPDO_SIZE & MAX_TXPDO_SIZE control in SOES.

For example you defined in slave:

#define MAX_RXPDO_SIZE   512
#define MAX_TXPDO_SIZE   512

Now you can map 600 bytes of objects to RXPRO and 600 bytes of objects to TXPDO by SOEM (dynamic PDO mapping).

When TXPDO_update() or RXPDO_update() functions are called they have no check of the size of 'rxpdo'/'txpdo' buffers, which cause buffer overflow in code: ESC_read (ESC_SM2_sma, rxpdo, ESCvar.ESC_SM2_sml); and ESC_write (ESC_SM3_sma, txpdo, ESCvar.ESC_SM3_sml);

Solution: check the size of mapped objects in transition PREOP_TO_SAFEOP/SAFEOP_TO_SAFEOP and the slave will not go to SAFEOP state in case of wrong PDO mapping.

    case PREOP_TO_SAFEOP:
    case SAFEOP_TO_SAFEOP:
    {
        ESCvar.ESC_SM2_sml = sizeOfPDO(RX_PDO_OBJIDX, &ESCvar.sm2mappings,
                                       SMmap2, MAX_MAPPINGS_SM2);
        if(ESCvar.sm2mappings < 0 || ESCvar.ESC_SM2_sml > MAX_RXPDO_SIZE)
        {
            an = ESCpreop | ESCerror;
            ESC_ALerror(ALERR_INVALIDOUTPUTSM);
            break;
        }

        ESCvar.ESC_SM3_sml = sizeOfPDO(TX_PDO_OBJIDX, &ESCvar.sm3mappings,
                                       SMmap3, MAX_MAPPINGS_SM3);
        if(ESCvar.sm3mappings < 0 || ESCvar.ESC_SM3_sml > MAX_TXPDO_SIZE)
        {
            an = ESCpreop | ESCerror;
            ESC_ALerror(ALERR_INVALIDINPUTSM);
            break;
        }
    }
nakarlsson commented 3 years ago

Thanks for input, the checks make sense. I know there are more checks that can be included. I'll mark this issue as an enhancement for future improvements that is planed.