* [ptxdist] [PATCH] libmodbus: fix remote buffer overflow vulnerabulity
@ 2013-02-04 11:26 Josef Holzmayr
2013-02-07 18:07 ` Michael Olbrich
0 siblings, 1 reply; 4+ messages in thread
From: Josef Holzmayr @ 2013-02-04 11:26 UTC (permalink / raw)
To: ptxdist; +Cc: Josef Holzmayr
[-- Attachment #1: Type: text/plain, Size: 5524 bytes --]
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.
+---
+ 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
[-- Attachment #2: Type: text/plain, Size: 48 bytes --]
--
ptxdist mailing list
ptxdist@pengutronix.de
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [ptxdist] [PATCH] libmodbus: fix remote buffer overflow vulnerabulity
2013-02-04 11:26 [ptxdist] [PATCH] libmodbus: fix remote buffer overflow vulnerabulity Josef Holzmayr
@ 2013-02-07 18:07 ` Michael Olbrich
2013-02-14 16:07 ` Josef Holzmayr
2013-04-04 8:41 ` Josef Holzmayr
0 siblings, 2 replies; 4+ messages in thread
From: Michael Olbrich @ 2013-02-07 18:07 UTC (permalink / raw)
To: ptxdist
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [ptxdist] [PATCH] libmodbus: fix remote buffer overflow vulnerabulity
2013-02-07 18:07 ` Michael Olbrich
@ 2013-02-14 16:07 ` Josef Holzmayr
2013-04-04 8:41 ` Josef Holzmayr
1 sibling, 0 replies; 4+ messages in thread
From: Josef Holzmayr @ 2013-02-14 16:07 UTC (permalink / raw)
To: ptxdist
Hi Michael,
Am 07.02.2013 19:07, schrieb Michael Olbrich:
>> +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?
you mean to s-o-b the patch itself? Sure, why not.
A real bugreport doesn't exist, just some bits and pieces:
- debian was not too concerned due to low count in popcon. We later
pushed the patch into the package, though.
- an earlier version of the patch was sent to the original maintainer
(as github pull request), he is a bit unresponsive though.
I guess the best will be to re-try and send it to upstream. Shall I keep
you in the loop then?
Greetz
_____________________________________________________________
Josef Holzmayr
Dipl-Ing. (FH)
Entwicklung Embedded Devices / Software
Tel.: +49 8444 9204-48>
Fax: +49 8444 9204-50
holzmayr@rsi-elektrotechnik.de
R-S-I Elektrotechnik GmbH & Co. KG
Woelkestrasse 11
D-85301 Schweitenkirchen
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ptxdist] [PATCH] libmodbus: fix remote buffer overflow vulnerabulity
2013-02-07 18:07 ` Michael Olbrich
2013-02-14 16:07 ` Josef Holzmayr
@ 2013-04-04 8:41 ` Josef Holzmayr
1 sibling, 0 replies; 4+ messages in thread
From: Josef Holzmayr @ 2013-04-04 8:41 UTC (permalink / raw)
To: ptxdist
Hello Michael,
Am 07.02.2013 19:07, schrieb Michael Olbrich:
> 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?
Coming back to this, upstream is still pretty unresponsive
(https://github.com/stephane/libmodbus/pull/105) and to be honest, i
don't feel like poking the maintainer anymore. If he doesn't care, he
doesn't care.
So for the good of the (obviously existing) libmodbus users in ptxdist,
please consider committing the patch. If you want, I can resend the
actual patch itself with my SOB.
_____________________________________________________________
Josef Holzmayr
Dipl-Ing. (FH)
Entwicklung Embedded Devices / Software
Tel.: +49 8444 9204-48>
Fax: +49 8444 9204-50
holzmayr@rsi-elektrotechnik.de
R-S-I Elektrotechnik GmbH & Co. KG
Woelkestrasse 11
D-85301 Schweitenkirchen
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-04-04 8:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-04 11:26 [ptxdist] [PATCH] libmodbus: fix remote buffer overflow vulnerabulity Josef Holzmayr
2013-02-07 18:07 ` Michael Olbrich
2013-02-14 16:07 ` Josef Holzmayr
2013-04-04 8:41 ` Josef Holzmayr
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox