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

Missing validation of request PDU parameters #112

Open
chengchangwu opened this issue Apr 11, 2022 · 5 comments
Open

Missing validation of request PDU parameters #112

chengchangwu opened this issue Apr 11, 2022 · 5 comments

Comments

@chengchangwu
Copy link

I've found that I can read at most 126 input registers with read_input_registers(). But why 126? Which part of the source code should I read.

@uklotzde
Copy link
Member

Limited by Modbus message size, please refer to the spec.

6.4 04 (0x04) Read Input Registers
This function code is used to read from 1 to 125 contiguous input registers in a remote device.

@chengchangwu
Copy link
Author

chengchangwu commented Apr 12, 2022

But I can read 126 contiguous registers. one more than the spec.

@uklotzde uklotzde added the bug label Mar 4, 2023
@uklotzde uklotzde changed the title Maximum number of input registers read Missing range checks of request PDU parameters Mar 4, 2023
@uklotzde
Copy link
Member

uklotzde commented Mar 4, 2023

This bug affects multiple function codes. Currently there are no semantic, client-side parameters according to the Modbus protocol validations in place. If a parameter value could technically be serialized it is accepted, even if the Modbus protocol specifies more restrictive, semantic bounds. Ideally servers should reject such non-standard requests.

Not a severe bug as clients are responsible for sending valid requests and servers are advised to reject invalid requests. Enforcing strict standard behavior by the library may even prevent using custom, vendor-specific extensions.

@uklotzde uklotzde changed the title Missing range checks of request PDU parameters Missing client-side validation of request PDU parameters Mar 4, 2023
@uklotzde uklotzde removed the bug label Mar 5, 2023
@uklotzde
Copy link
Member

uklotzde commented Mar 5, 2023

Not a bug, it's a "feature".

If those additional conformance checks are implemented then they should be hidden behind a feature flag. Not everyone might need or want them.

@uklotzde uklotzde changed the title Missing client-side validation of request PDU parameters Missing validation of request PDU parameters Mar 6, 2023
@uklotzde
Copy link
Member

uklotzde commented Mar 6, 2023

This affects both the client and the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants