From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from ptx.hi.pengutronix.de ([2001:6f8:1178:2:5054:ff:fec0:8e10] ident=Debian-exim) by metis.ext.pengutronix.de with esmtp (Exim 4.72) (envelope-from ) id 1U3Vt9-0005vf-Ai for ptxdist@pengutronix.de; Thu, 07 Feb 2013 19:07:59 +0100 Received: from mol by ptx.hi.pengutronix.de with local (Exim 4.72) (envelope-from ) id 1U3Vt9-0004I8-AP for ptxdist@pengutronix.de; Thu, 07 Feb 2013 19:07:59 +0100 Date: Thu, 7 Feb 2013 19:07:59 +0100 From: Michael Olbrich Message-ID: <20130207180759.GC6194@pengutronix.de> References: <1359977168-20083-1-git-send-email-holzmayr@rsi-elektrotechnik.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1359977168-20083-1-git-send-email-holzmayr@rsi-elektrotechnik.de> Subject: Re: [ptxdist] [PATCH] libmodbus: fix remote buffer overflow vulnerabulity Reply-To: ptxdist@pengutronix.de List-Id: PTXdist Development Mailing List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: ptxdist-bounces@pengutronix.de Errors-To: ptxdist-bounces@pengutronix.de To: ptxdist@pengutronix.de On Mon, Feb 04, 2013 at 12:26:08PM +0100, Josef Holzmayr wrote: > Signed-off-by: Josef Holzmayr > --- > ...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 > +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