Comment 6 for bug 11223

Message-ID: <email address hidden>
Date: Tue, 21 Dec 2004 16:26:00 +0100
From: Hans Kratz <email address hidden>
To: <email address hidden>
Cc: <email address hidden>, <email address hidden>
Subject: [PATCH,RFC] Backport of PHP 4.3.9 security fixes: pack()/unpack()

------=_Part_305_25427426.1103642760216
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Hi!

I am not a Debian developer but we use Woody on our servers. The
latest PHP security holes affect us as well. I have backported the
security fix for the pack()/unpack() functions (attached).

Attached patch is against PHP 4.1.2-7. PHP 4.1.2-7+patch builds fine
in a Woody pbuilder and looks ok but I have not yet otherwise tested
it.

Comments? Should I try to backport the other security fixes as well?

Regards,

Hans
--
Hans Kratz

------=_Part_305_25427426.1103642760216
Content-Type: text/x-patch; name=php4-4.1.2-7-pack-unpack-fix.patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="php4-4.1.2-7-pack-unpack-fix.patch"

diff -purN php4-4.1.2-7.orig/ext/standard/pack.c php4-4.1.2-7/ext/standard/pack.c
--- php4-4.1.2-7.orig/ext/standard/pack.c 2001-08-11 19:03:37.000000000 +0200
+++ php4-4.1.2-7/ext/standard/pack.c 2004-12-19 14:55:26.000000000 +0100
@@ -49,6 +49,13 @@
 #include <netinet/in.h>
 #endif

+#define INC_OUTPUTPOS(a,b) \
+ if ((a) < 0 || ((INT_MAX - outputpos)/(b)) < (a)) { \
+ php_error(E_WARNING, "Type %c: integer overflow in format string", code); \
+ RETURN_FALSE; \
+ } \
+ outputpos += (a)*(b);
+
 /* Whether machine is little endian */
 char machine_little_endian;

@@ -216,39 +223,39 @@ PHP_FUNCTION(pack)

   switch ((int)code) {
    case 'h': case 'H': {
- outputpos += (arg + 1) / 2; /* 4 bit per arg */
+ INC_OUTPUTPOS((arg + 1) / 2,1) /* 4 bit per arg */
     break;
    }

    case 'a': case 'A':
    case 'c': case 'C':
    case 'x': {
- outputpos += arg; /* 8 bit per arg */
+ INC_OUTPUTPOS(arg,1) /* 8 bit per arg */
     break;
    }

    case 's': case 'S': case 'n': case 'v': {
- outputpos += arg * 2; /* 16 bit per arg */
+ INC_OUTPUTPOS(arg,2) /* 16 bit per arg */
     break;
    }

    case 'i': case 'I': {
- outputpos += arg * sizeof(int);
+ INC_OUTPUTPOS(arg,sizeof(int))
     break;
    }

    case 'l': case 'L': case 'N': case 'V': {
- outputpos += arg * 4; /* 32 bit per arg */
+ INC_OUTPUTPOS(arg,4) /* 32 bit per arg */
     break;
    }

    case 'f': {
- outputpos += arg * sizeof(float);
+ INC_OUTPUTPOS(arg,sizeof(float))
     break;
    }

    case 'd': {
- outputpos += arg * sizeof(double);
+ INC_OUTPUTPOS(arg,sizeof(double))
     break;
    }

@@ -615,6 +622,11 @@ PHP_FUNCTION(unpack)
     sprintf(n, "%.*s", namelen, name);
    }

+ if (size != 0 && size != -1 && INT_MAX - size + 1 < inputpos) {
+ php_error(E_WARNING, "Type %c: integer overflow", type);
+ inputpos = 0;
+ }
+
    if ((inputpos + size) <= inputlen) {
     switch ((int)type) {
      case 'a': case 'A': {
@@ -778,6 +790,10 @@ PHP_FUNCTION(unpack)
     }

     inputpos += size;
+ if (inputpos < 0) {
+ php_error(E_WARNING, "Type %c: outside of string", type);
+ inputpos = 0;
+ }
    } else if (arg < 0) {
     /* Reached end of input for '*' repeater */
     break;

------=_Part_305_25427426.1103642760216--