Skip to content

Fix vulnerabilities reported in October 2026 - #243

Merged
lategoodbye merged 13 commits into
rscada:masterfrom
lategoodbye:bugfixes
Oct 20, 2026
Merged

lategoodbye merged 13 commits into
rscada:masterfrom
lategoodbye:bugfixes

Conversation

@lategoodbye

@lategoodbye lategoodbye commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

This pull request fix the following vulnerabilities for this month:

  • Properly check result of gmtime
  • Fix out-of-bounds reads in mbus_data_variable_parse & mbus_data_bcd_decode
  • Fix memory leaks in mbus_data_variable_parse & mbus_parse_variable_record
  • Fix shift behavior in mbus_data_record_storage_number
  • Fix mbus_frame_print for huge frames
  • Prevent shift overflow in mbus_data_bcd_decode(_hex)
  • Fix double-free in mbus_frame_xml
  • Add missing range check for ndife & nvife & data_len
  • Properly handle mbus_frame_pack result

Thanks to Lee Pike & Tony Gair & johvik for reporting.

@lategoodbye

Copy link
Copy Markdown
Collaborator Author

@johvik Maybe you want to take a look at it before I merge?

@johvik

johvik commented Oct 16, 2026 via email •

Copy link
Copy Markdown
Contributor

@lategoodbye
lategoodbye force-pushed the bugfixes branch 2 times, most recently from 42e881f to b85ee39 Compare October 18, 2026 12:17

@johvik johvik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lategoodbye I think it looks pretty good overall, have two concerns/questions in mbus_data_variable_parse() and some other minor thoughts/ideas

Comment thread mbus/mbus-protocol.c Resolved
Comment thread mbus/mbus-protocol.c Resolved
Comment thread mbus/mbus-protocol.c Resolved
Comment thread mbus/mbus-protocol.c Resolved
Comment thread mbus/mbus-protocol.c Resolved
Comment thread mbus/mbus-protocol.c Resolved
@lategoodbye

Copy link
Copy Markdown
Collaborator Author

@lategoodbye I think it looks pretty good overall, have two concerns/questions in mbus_data_variable_parse() and some other minor thoughts/ideas

I think all your comments should be addressed now

@johvik johvik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last thing, looks good to me apart from that 👍

Comment thread mbus/mbus-protocol.c
Comment on lines +4020 to +4025
if (i >= frame->data_size)
{
mbus_data_record_free(record);
snprintf(error_str, sizeof(error_str), "Premature end of record at LVAR.");
return -1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is missing mbus_data_record_free(data->record); data->record = NULL;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh dear, thanks

lategoodbye and others added 13 commits October 20, 2026 12:22
Issues has been found with Codex
At two places in mbus_data_variable_parse were no proper bounds checks,
which could result in a one-byte out-of-bounds reads.

Thanks to Tony Gair for reporting.
In error cases the function leaked memory of the successful parsed M-Bus
data records.

Thanks to Lee Pike for reporting.
Some function assume ndife & nvife always to be in a valid range.
So avoid a possible out of bounds access.

Issue has been found with Codex
Huge M-Bus frames are not properly handled by mbus_frame_print, which could
result in dump of uninitialized memory.
These helper functions can overflow for big BCD numbers, but weren't
intended for external use. So limit it to the libmbus use case.

While at this fix an off-by-one in mbus_data_bcd_decode.

Thanks to Tony Gair for reporting.
After freeing the records of the last frame the pointer should be
set to NULL. Otherwise this could result in a double-free.

Issue has been found with Codex
In case mbus_parse_variable_record fails to normalize the unit, the
allocate string should also be freed.
The shift calculation doesn't properly handle big storage numbers.
Since datatype long isn't capable to store the maximum on all platforms,
this should also be handled as an error.

Thanks to Lee Pike and Tony Gair for reporting.
Some functions assume data_len always to be in a valid range.
So avoid a possible out of bounds access.

Issue has been found with Codex
@lategoodbye
lategoodbye merged commit e4b6b05 into rscada:master Oct 20, 2026
2 checks passed
@lategoodbye
lategoodbye deleted the bugfixes branch October 26, 2026 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants