catalinv-ncc opened a new issue, #19260:
URL: https://github.com/apache/nuttx/issues/19260

   ### Description / Steps to reproduce the issue
   
   drivers\contactless\mfrc522: Contactless Driver Is Not Robust
   
   # Impact
   
   An attacker can specify an arbitrary page address when reading MIFARE tags. 
Without validation, this could read beyond intended memory regions on the tag, 
potentially causing a crash.
   
   # Description
   
   Aside from the fact that user space is not validated (see finding 
[finding:RP6]) the `mfrc522_mifare_read()` command is also not robust and does 
not check that the page address is valid. From section 7.6.5 of the *MIFARE 
Ultralight contactless single-ticket IC 
(https://www.nxp.com/docs/en/data-sheet/MF0ICU1.pdf) document:
   
   > *The READ command needs the page address as a parameter. Only addresses 
00h to 0Fh are decoded.*
   
   In the snippet below the 4-byte command is created `CMD ADDRESS CRC0 CRC1` 
and it is sent to the MFRC522 IC.
   
   ```c
   static int mfrc522_ioctl(FAR struct file *filep, int cmd, **unsigned long 
arg**)
   {
   ...
             FAR struct mifare_tag_data_s ***data** =
                                          (struct mifare_tag_data_s *)**arg**;
             ret = **mfrc522_mifare_read**(dev, **data**);
   ...
   }
   
   int mfrc522_mifare_read(FAR struct mfrc522_dev_s *dev,
                           [[[[FAR struct mifare_tag_data_s *data]]]])
   {
     uint8_t buffer[18];
     uint8_t command[4];
     uint8_t length    = 18;
   ...
     command[0] = PICC_CMD_MF_READ; /* NCC Group: 0x30 */
     [[[[command[1] = data->address;]]]]
   
     /* Get CRC */
     ret = mfrc522_calc_crc(dev, command, 2, &command[2]);
   ...
     ret = [[[[mfrc522_transcv_data]]]](dev, [[[[command]]]], 4, buffer, 
&length,
                                &validbits, 0, false);
     if (ret < 0)
       {
         goto errout;
       }
     memcpy(data->data, buffer, 16);
   
   errout:
     return ret;
   }
   
   int [[[[mfrc522_transcv_data]]]](FAR struct mfrc522_dev_s *dev,
                            [[[[uint8_t *senddata]]]], uint8_t sendlen,
                            uint8_t *backdata, uint8_t *backlen,
                            uint8_t *validbits, uint8_t rxalign,
                            bool check_crc)
   {
   ...
     return [[[[mfrc522_comm_picc]]]](dev, MFRC522_TRANSCV_CMD, waitirq, 
[[[[senddata]]]],
                              sendlen, backdata, backlen,
                              validbits, rxalign, check_crc);
   }
   
   int mfrc522_comm_picc(FAR struct mfrc522_dev_s *dev, uint8_t command,
                         uint8_t waitirq, [[[[uint8_t *send_data,]]]] uint8_t 
send_len,
                         uint8_t *back_data, uint8_t *back_len,
                         uint8_t *validbits, uint8_t rxalign, bool checkcrc)
   {
   ...
     [[[[mfrc522_writeblk]]]](dev, MFRC522_FIFO_DATA_REG, [[[[send_data]]]], 
send_len);
   ...
   }
   ~~~
   
   It is not clear how the IC reacts to bad input, therefore this finding was 
reported due to an abundance of caution, and it is rated as Informational.
   
   ~~~
   void [[[[mfrc522_writeblk]]]](FAR struct mfrc522_dev_s *dev, uint8_t regaddr,
                         [[[[uint8_t *regval]]]], int length)
   {
   ...
     SPI_SNDBLOCK(dev->spi, [[[[regval]]]], length);
   ...
   }
   ~~~
   
   A similar issue can be found in the PN532 driver:
   
   ~~~c
   static int _ioctl(FAR struct file *filep, int cmd, [[[[unsigned long arg]]]])
   {
   ...
     switch (cmd)
       {
       case PN532IOC_READ_TAG_DATA:
         {
           FAR struct pn_mifare_tag_data_s [[[[*tag_data]]]] =
             (FAR struct pn_mifare_tag_data_s *) [[[[arg]]]];
   ...
               ret = pn532_read_passive_data(dev, [[[[tag_data->address]]]],
                                             (FAR uint8_t *)&tag_data->data,
                                             sizeof(tag_data->data));
   
   uint32_t pn532_read_passive_data(FAR struct pn532_dev_s *dev,
                                    [[[[uint8_t address]]]], FAR uint8_t *data,
                                    uint8_t len)
   {
   ...
     pn532_frame_init(f, PN532_COMMAND_INDATAEXCHANGE);
     f->data[1] = 1;       /* max n cards at once */
     f->data[2] = 0x30;    /* command READ */
     [[[[f->data[3] = address;]]]] /* ADDRESS, 0 = serial */
   ~~~
   
   # Recommendation
   
   NuttX driver should not rely on the IC to behave correctly and test the 
validity of the user-provided parameters before sending the read command to the 
RFID IC:
   
   ```c
     /* For MFRC522 */
     if (data->address > 15)
       {
         /* only pages 0x0, ..., 0xf are valid. */
         ret = -ERANGE;
         goto errout;
       }
   
     /* For PN532 */
     if (address >= 16)  /* Validate against expected tag size */
       {
         return -ERANGE;
       }
   ```
   
   ### On which OS does this issue occur?
   
   [OS: Linux]
   
   ### What is the version of your OS?
   
   Ubuntu 24.04
   
   ### NuttX Version
   
   master
   
   ### Issue Architecture
   
   [Arch: all]
   
   ### Issue Area
   
   [Area: Drivers]
   
   ### Host information
   
   _No response_
   
   ### Verification
   
   - [x] I have verified before submitting the report.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to