[PATCH 2/3] udev_rules_apply_format(): don't overflow the buffer when

[PATCH 2/3] udev_rules_apply_format(): don't overflow the buffer when

[Sergey Vlasov]

When truncating the substitution string to the length specified in the
format string, head[len] = '\0' could write outside the buffer if that
length was too large.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
---
 udev_rules.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/udev_rules.c b/udev_rules.c
index 44b41e9..90a83e5 100644
--- a/udev_rules.c
+++ b/udev_rules.c
@@ -558,7 +558,7 @@ found:
 			break;
 		}
 		/* possibly truncate to format-char specified length */
-		if (len != -1) {
+		if (len >= 0 && (size_t)len < maxsize - (head-string)) {
 			head[len] = '\0';
 			dbg("truncate to %i chars, subtitution string becomes '%s'", len, head);
 		}
-- 
1.5.0.rc2.gd894fb


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x120709&bid&3057&dat\x121642
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH 2/3] udev_rules_apply_format(): don't overflow the buffer

[Kay Sievers]

On 2/1/07, Sergey Vlasov <vsu@altlinux.ru> wrote:
> When truncating the substitution string to the length specified in the
> format string, head[len] = '\0' could write outside the buffer if that
> length was too large.
>
> Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
> ---
>  udev_rules.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/udev_rules.c b/udev_rules.c
> index 44b41e9..90a83e5 100644
> --- a/udev_rules.c
> +++ b/udev_rules.c
> @@ -558,7 +558,7 @@ found:
>                         break;
>                 }
>                 /* possibly truncate to format-char specified length */
> -               if (len != -1) {
> +               if (len >= 0 && (size_t)len < maxsize - (head-string)) {

Yeah, that should be fixed. Wouldn't:
  if (len >= 0 && len <= strlen(head))
also catch strings which fit in the buffer, but are shorter than the
specified limit?

Thanks,
Kay

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x120709&bid&3057&dat\x121642
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH 2/3] udev_rules_apply_format(): don't overflow the buffer

[Sergey Vlasov]

[-- Attachment #1.1: Type: text/plain, Size: 1232 bytes --]

On Thu, Feb 01, 2007 at 06:18:16PM +0100, Kay Sievers wrote:
> On 2/1/07, Sergey Vlasov <vsu@altlinux.ru> wrote:
> >When truncating the substitution string to the length specified in the
> >format string, head[len] = '\0' could write outside the buffer if that
> >length was too large.
> >
> >Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
> >---
> > udev_rules.c |    2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> >diff --git a/udev_rules.c b/udev_rules.c
> >index 44b41e9..90a83e5 100644
> >--- a/udev_rules.c
> >+++ b/udev_rules.c
> >@@ -558,7 +558,7 @@ found:
> >                        break;
> >                }
> >                /* possibly truncate to format-char specified length */
> >-               if (len != -1) {
> >+               if (len >= 0 && (size_t)len < maxsize - (head-string)) {
> 
> Yeah, that should be fixed. Wouldn't:
>  if (len >= 0 && len <= strlen(head))
> also catch strings which fit in the buffer, but are shorter than the
> specified limit?

Yes, this should work properly too (or even "&& len < strlen(head)" to
skip writing '\0' over itself).  Working in constant time is probably
not an issue here, given the number of strlcat() calls around.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 374 bytes --]

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

[-- Attachment #3: Type: text/plain, Size: 226 bytes --]

_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH 2/3] udev_rules_apply_format(): don't overflow the

[Kay Sievers]

On Thu, 2007-02-01 at 22:09 +0300, Sergey Vlasov wrote:
> On Thu, Feb 01, 2007 at 06:18:16PM +0100, Kay Sievers wrote:
> > On 2/1/07, Sergey Vlasov <vsu@altlinux.ru> wrote:
> > >When truncating the substitution string to the length specified in the
> > >format string, head[len] = '\0' could write outside the buffer if that
> > >length was too large.
> > >
> > >Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
> > >---
> > > udev_rules.c |    2 +-
> > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > >diff --git a/udev_rules.c b/udev_rules.c
> > >index 44b41e9..90a83e5 100644
> > >--- a/udev_rules.c
> > >+++ b/udev_rules.c
> > >@@ -558,7 +558,7 @@ found:
> > >                        break;
> > >                }
> > >                /* possibly truncate to format-char specified length */
> > >-               if (len != -1) {
> > >+               if (len >= 0 && (size_t)len < maxsize - (head-string)) {
> > 
> > Yeah, that should be fixed. Wouldn't:
> >  if (len >= 0 && len <= strlen(head))
> > also catch strings which fit in the buffer, but are shorter than the
> > specified limit?
> 
> Yes, this should work properly too (or even "&& len < strlen(head)" to
> skip writing '\0' over itself).  Working in constant time is probably
> not an issue here, given the number of strlcat() calls around.

I've applied this together with the other patch, which logs an error for
unknown substitutions.

Thanks a lot,
Kay


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x120709&bid&3057&dat\x121642
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel