From: Michael Olbrich <m.olbrich@pengutronix.de>
To: ptxdist@pengutronix.de
Subject: Re: [ptxdist] [PATCH] libmodbus: fix remote buffer overflow vulnerabulity
Date: Thu, 7 Feb 2013 19:07:59 +0100 [thread overview]
Message-ID: <20130207180759.GC6194@pengutronix.de> (raw)
In-Reply-To: <1359977168-20083-1-git-send-email-holzmayr@rsi-elektrotechnik.de>
On Mon, Feb 04, 2013 at 12:26:08PM +0100, Josef Holzmayr wrote:
> Signed-off-by: Josef Holzmayr <holzmayr@rsi-elektrotechnik.de>
> ---
> ...Add-register-count-checks-to-modbus_reply.patch | 90 ++++++++++++++++++++++
> patches/libmodbus-3.0.3/series | 1 +
> 2 files changed, 91 insertions(+)
> create mode 100644 patches/libmodbus-3.0.3/0001-Add-register-count-checks-to-modbus_reply.patch
> create mode 100644 patches/libmodbus-3.0.3/series
>
> diff --git a/patches/libmodbus-3.0.3/0001-Add-register-count-checks-to-modbus_reply.patch b/patches/libmodbus-3.0.3/0001-Add-register-count-checks-to-modbus_reply.patch
> new file mode 100644
> index 0000000..2a1ea17
> --- /dev/null
> +++ b/patches/libmodbus-3.0.3/0001-Add-register-count-checks-to-modbus_reply.patch
> @@ -0,0 +1,90 @@
> +From 58cf8959b0b5067cd63f23d44f338e41fa7c116a Mon Sep 17 00:00:00 2001
> +From: Josef Holzmayr <holzmayr@rsi-elektrotechnik.de>
> +Date: Thu, 22 Sep 2011 14:31:39 +0200
> +Subject: [PATCH] Add register count checks to modbus_reply
> +
> +Add checks so modbus_reply returns a
> +MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS if the count of requested
> +registers exceeds the spec as noted in modbus.h, line 73ff.
Can you add your s-o-b here? This looks like it is something for upstream.
Is there some bug report or something that you can reference here, so we
know what to do with this when the next update happens?
Michael
> +---
> + src/modbus.c | 22 +++++++++++++++-------
> + 1 files changed, 15 insertions(+), 7 deletions(-)
> +
> +diff --git a/src/modbus.c b/src/modbus.c
> +index 2860d29..64b9d92 100644
> +--- a/src/modbus.c
> ++++ b/src/modbus.c
> +@@ -662,7 +662,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req,
> + case _FC_READ_COILS: {
> + int nb = (req[offset + 3] << 8) + req[offset + 4];
> +
> +- if ((address + nb) > mb_mapping->nb_bits) {
> ++ if ((address + nb) > mb_mapping->nb_bits ||
> ++ nb > MODBUS_MAX_READ_REGISTERS) {
> + if (ctx->debug) {
> + fprintf(stderr, "Illegal data address %0X in read_bits\n",
> + address + nb);
> +@@ -684,7 +685,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req,
> + * function) */
> + int nb = (req[offset + 3] << 8) + req[offset + 4];
> +
> +- if ((address + nb) > mb_mapping->nb_input_bits) {
> ++ if ((address + nb) > mb_mapping->nb_input_bits ||
> ++ nb > MODBUS_MAX_READ_REGISTERS) {
> + if (ctx->debug) {
> + fprintf(stderr, "Illegal data address %0X in read_input_bits\n",
> + address + nb);
> +@@ -704,7 +706,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req,
> + case _FC_READ_HOLDING_REGISTERS: {
> + int nb = (req[offset + 3] << 8) + req[offset + 4];
> +
> +- if ((address + nb) > mb_mapping->nb_registers) {
> ++ if ((address + nb) > mb_mapping->nb_registers ||
> ++ nb > MODBUS_MAX_READ_REGISTERS) {
> + if (ctx->debug) {
> + fprintf(stderr, "Illegal data address %0X in read_registers\n",
> + address + nb);
> +@@ -729,7 +732,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req,
> + * function) */
> + int nb = (req[offset + 3] << 8) + req[offset + 4];
> +
> +- if ((address + nb) > mb_mapping->nb_input_registers) {
> ++ if ((address + nb) > mb_mapping->nb_input_registers ||
> ++ nb > MODBUS_MAX_READ_REGISTERS) {
> + if (ctx->debug) {
> + fprintf(stderr, "Illegal data address %0X in read_input_registers\n",
> + address + nb);
> +@@ -797,7 +801,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req,
> + case _FC_WRITE_MULTIPLE_COILS: {
> + int nb = (req[offset + 3] << 8) + req[offset + 4];
> +
> +- if ((address + nb) > mb_mapping->nb_bits) {
> ++ if ((address + nb) > mb_mapping->nb_bits ||
> ++ nb > MODBUS_MAX_WRITE_REGISTERS) {
> + if (ctx->debug) {
> + fprintf(stderr, "Illegal data address %0X in write_bits\n",
> + address + nb);
> +@@ -819,7 +824,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req,
> + case _FC_WRITE_MULTIPLE_REGISTERS: {
> + int nb = (req[offset + 3] << 8) + req[offset + 4];
> +
> +- if ((address + nb) > mb_mapping->nb_registers) {
> ++ if ((address + nb) > mb_mapping->nb_registers ||
> ++ nb > MODBUS_MAX_WRITE_REGISTERS) {
> + if (ctx->debug) {
> + fprintf(stderr, "Illegal data address %0X in write_registers\n",
> + address + nb);
> +@@ -873,7 +879,9 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req,
> + int nb_write = (req[offset + 7] << 8) + req[offset + 8];
> +
> + if ((address + nb) > mb_mapping->nb_registers ||
> +- (address_write + nb_write) > mb_mapping->nb_registers) {
> ++ (address_write + nb_write) > mb_mapping->nb_registers ||
> ++ nb > MODBUS_MAX_RW_WRITE_REGISTERS ||
> ++ nb_write > MODBUS_MAX_RW_WRITE_REGISTERS) {
> + if (ctx->debug) {
> + fprintf(stderr,
> + "Illegal data read address %0X or write address %0X write_and_read_registers\n",
> +--
> +1.7.4.1
> +
> diff --git a/patches/libmodbus-3.0.3/series b/patches/libmodbus-3.0.3/series
> new file mode 100644
> index 0000000..77fe122
> --- /dev/null
> +++ b/patches/libmodbus-3.0.3/series
> @@ -0,0 +1 @@
> +0001-Add-register-count-checks-to-modbus_reply.patch
> --
> 1.8.1.1
>
>
> --
> _____________________________________________________________
> R-S-I Elektrotechnik GmbH & Co. KG
> Woelkestrasse 11
> D-85301 Schweitenkirchen
> Fon: +49 8444 9204-0
> Fax: +49 8444 9204-50
> www.rsi-elektrotechnik.de
>
> _____________________________________________________________
> Amtsgericht Ingolstadt - GmbH: HRB 191328 - KG: HRA 170363
> Gesch?ftsf?hrer: Dr.-Ing. Michael Sorg, Dipl.-Ing. Franz Sorg
> USt-IdNr.: DE 128592548
>
>
> --
> ptxdist mailing list
> ptxdist@pengutronix.de
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
ptxdist mailing list
ptxdist@pengutronix.de
next prev parent reply other threads:[~2013-02-07 18:07 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-04 11:26 Josef Holzmayr
2013-02-07 18:07 ` Michael Olbrich [this message]
2013-02-14 16:07 ` Josef Holzmayr
2013-04-04 8:41 ` Josef Holzmayr
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130207180759.GC6194@pengutronix.de \
--to=m.olbrich@pengutronix.de \
--cc=ptxdist@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox