Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems with RPDO dummy mapping - potential array out of bounds write #138

Open
Florian-2718 opened this issue Mar 9, 2023 · 2 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Florian-2718
Copy link

Hello,
The functions "CORPdoGetMap()" and "CORPdoWrite()" show potentially uncorrect behaviour when RPDO dummy mapping is used.
In the CiA 301 document there is stated for the Receive PDO Mapping Parameter (1600h - 17FFh) that if
data types (1h..7h) are mapped, then these entries serve as dummy entries and the corresponding data is not evaluated by the device.

The EO CANOpen stack supports this feature, see functions "CORPdoGetMap()" and "CORPdoWrite()".

But in the named functions I recognized the following problems:

  • "CORPdoGetMap()":
    Let's assume there are mapped 8 objects:
    Subindex 1: 0x00020008 - Dummy entry with length 1 Byte
    Subindex 2: 0x00020008 - Dummy entry with length 1 Byte
    Subindex 3: 0x00020008 - Dummy entry with length 1 Byte
    Subindex 4: 0x00020008 - Dummy entry with length 1 Byte
    Subindex 5: 0x00020008 - Dummy entry with length 1 Byte
    Subindex 6: 0x00020008 - Dummy entry with length 1 Byte
    Subindex 7: 0x00020008 - Dummy entry with length 1 Byte
    Subindex 8: 0x60400008 - Length 1 Byte , reference to index 0x6040
    ==> Then the access to "pdo[num].Map[]" and to "pdo[num].Size[]" will be performed with index 14 ("on + dummy") for
    the subindex 8, where index 7 is the highest valid subindex of the array "pdo".
    This would equal a array out of bounds write action.
  • "CORPdoWrite()":
    Let's assume that between two "normal" entries a dummy entry exists, then for this dummy entry the variable "dlc"
    won't be incremented and afterwards the access regarding the CAN-frame will be performed on the wrong (too low) indices.

I hope my assumtions are correct.

Best regards,
Florian

@mrk-its
Copy link

mrk-its commented May 21, 2023

I had to fix this code on my branch this way: mrk-its@3818dee

@michael-hillmann michael-hillmann added the bug Something isn't working label Sep 7, 2023
@michael-hillmann michael-hillmann added this to the 4.4.1 milestone Sep 7, 2023
@michael-hillmann
Copy link
Contributor

You are absolutely right! Thanks for this issue, we will write tests for some RPDO-Dummy use-cases and use the mentioned fix afterwards.

@michael-hillmann michael-hillmann self-assigned this Sep 8, 2023
sandrogort pushed a commit to sandrogort/canopen-stack that referenced this issue Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants