Skip to content

Fix exception function code wraparound when function >= 0x80 #873

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.

Sign up for GitHub

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

Closed
afonsojanu wants to merge 1 commit into from
Closed

Fix exception function code wraparound when function >= 0x80 #873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/modbus.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,8 +785,17 @@ static int response_exception(modbus_t *ctx,
modbus_flush(ctx);
}

/* Build exception response */
sft->function = sft->function + 0x80;
/* Build exception response. Per the Modbus spec the exception function
code is formed by setting the top bit of the request's function code.
An unrecognized function code can already have that bit set (any raw
byte from 0x80 to 0xFF ends up here through the "default" case in
modbus_reply's switch), so adding 0x80 can carry past 0xFF and the
truncation to a single byte in build_response_basis then wraps the
result back below 0x80. That leaves the response looking like a
plain, non-exception function code to the client. A bitwise OR gives
the same result as the addition when the top bit was clear and stays
correct when it was already set. */
sft->function = sft->function | 0x80;
rsp_length = ctx->backend->build_response_basis(sft, rsp);
rsp[rsp_length++] = exception_code;

Expand Down
20 changes: 20 additions & 0 deletions tests/unit-test-client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,10 @@ int test_server(modbus_t *ctx, int use_backend)
const int INVALID_FC = 0x42;
const int INVALID_FC_REQ_LEN = 6;
uint8_t invalid_fc_raw_req[] = {slave, 0x42, 0x00, 0x00, 0x00, 0x00};
/* Same idea, but the function byte already has its top bit set, as a
real function code >= 0x80 would. */
const int INVALID_FC_HIGH = 0xFF;
uint8_t invalid_fc_high_raw_req[] = {slave, 0xFF, 0x00, 0x00, 0x00, 0x00};

int req_length;
uint8_t rsp[MODBUS_TCP_MAX_ADU_LENGTH];
Expand Down Expand Up @@ -999,6 +1003,22 @@ int test_server(modbus_t *ctx, int use_backend)
rsp[backend_offset] == (0x80 + INVALID_FC),
"")

/* Same test with a function code that already has its top bit set. The
server used to build the exception response by adding 0x80 to the
function code with plain arithmetic, so a request byte >= 0x80 made
the sum carry past 0xFF; once that got truncated to a single byte for
the wire, the result wrapped back below 0x80 and looked like a
perfectly normal function code to the client instead of an
exception. */
modbus_send_raw_request(
ctx, invalid_fc_high_raw_req, INVALID_FC_REQ_LEN * sizeof(uint8_t));
rc = modbus_receive_confirmation(ctx, rsp);
printf("Return an exception on unknown function code with the top bit "
"already set: ");
ASSERT_TRUE(rc == (backend_length + EXCEPTION_RC) &&
rsp[backend_offset] == (0x80 | INVALID_FC_HIGH),
"")

modbus_set_response_timeout(ctx, old_response_to_sec, old_response_to_usec);
return 0;
close:
Expand Down