The Samba-Bugzilla – Attachment 7392 Details for
Bug 8815
PIDL based autogenerated code allows overwriting beyond of allocated array; CVE-2012-1182
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
draft patches for master
pidl-array-length-03.master.txt (text/plain), 16.80 KB, created by
Stefan Metzmacher
on 2012-03-15 18:03:33 UTC
(
hide
)
Description:
draft patches for master
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2012-03-15 18:03:33 UTC
Size:
16.80 KB
patch
obsolete
>From 59bdb488f4894e10d5a722161277d69f810bddc1 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 15 Mar 2012 13:09:51 +0100 >Subject: [PATCH 1/8] pidl/NDR/Parser: declare all union helper variables in ParseUnionPull() > >metze >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index 16ed685..d8cf974 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -1940,8 +1940,6 @@ sub ParseUnionPullPrimitives($$$$$) > > if ($el->{TYPE} ne "EMPTY") { > $self->indent; >- $self->DeclarePtrVariables($el); >- $self->DeclareArrayVariables($el); > if (defined($e->{PROPERTIES}{relative_base})) { > $self->pidl("NDR_CHECK(ndr_pull_align($ndr, $el->{ALIGN}));"); > # set the current offset as base for relative pointers >@@ -2018,6 +2016,8 @@ sub ParseUnionPull($$$$) > next if ($el->{TYPE} eq "EMPTY"); > next if ($double_cases{"$el->{NAME}"}); > $self->DeclareMemCtxVariables($el); >+ $self->DeclarePtrVariables($el); >+ $self->DeclareArrayVariables($el); > $double_cases{"$el->{NAME}"} = 1; > } > >-- >1.7.4.1 > > >From 25c6df97ee45421105f99352fa6efff838fba91c Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 15 Mar 2012 13:12:04 +0100 >Subject: [PATCH 2/8] pidl/NDR/Parser: simplify logic in DeclareArrayVariables*() > >metze >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 14 ++++++-------- > 1 files changed, 6 insertions(+), 8 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index d8cf974..709ad2a 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -1583,11 +1583,10 @@ sub DeclareArrayVariables($$) > my ($self,$e) = @_; > > foreach my $l (@{$e->{LEVELS}}) { >+ next if ($l->{TYPE} ne "ARRAY"); > next if has_fast_array($e,$l); > next if is_charset_array($e,$l); >- if ($l->{TYPE} eq "ARRAY") { >- $self->pidl("uint32_t cntr_$e->{NAME}_$l->{LEVEL_INDEX};"); >- } >+ $self->pidl("uint32_t cntr_$e->{NAME}_$l->{LEVEL_INDEX};"); > } > } > >@@ -1596,15 +1595,14 @@ sub DeclareArrayVariablesNoZero($$$) > my ($self,$e,$env) = @_; > > foreach my $l (@{$e->{LEVELS}}) { >+ next if ($l->{TYPE} ne "ARRAY"); > next if has_fast_array($e,$l); > next if is_charset_array($e,$l); >- if ($l->{TYPE} eq "ARRAY") { >- my $length = ParseExpr($l->{LENGTH_IS}, $env, $e->{ORIGINAL}); >- if ($length eq "0") { >+ my $length = ParseExpr($l->{LENGTH_IS}, $env, $e->{ORIGINAL}); >+ if ($length eq "0") { > warning($e->{ORIGINAL}, "pointless array cntr: 'cntr_$e->{NAME}_$l->{LEVEL_INDEX}': length=$length"); >- } else { >+ } else { > $self->pidl("uint32_t cntr_$e->{NAME}_$l->{LEVEL_INDEX};"); >- } > } > } > } >-- >1.7.4.1 > > >From cd49adb7f1bb7421feb3a83c7c6e451004850f24 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 15 Mar 2012 13:05:39 +0100 >Subject: [PATCH 3/8] pidl/NDR/Parser: split off ParseArrayPullGetSize() and ParseArrayPullGetLength() > >metze >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 55 +++++++++++++++++++++++------- > 1 files changed, 42 insertions(+), 13 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index 709ad2a..eaf673b 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -321,39 +321,68 @@ sub check_null_pointer($$$$) > } > } > >-##################################################################### >-# parse an array - pull side >-sub ParseArrayPullHeader($$$$$$) >+sub ParseArrayPullGetSize($$$$$$) > { > my ($self,$e,$l,$ndr,$var_name,$env) = @_; > >- my $length; > my $size; > > if ($l->{IS_CONFORMANT}) { >- $length = $size = "ndr_get_array_size($ndr, " . get_pointer_to($var_name) . ")"; >+ $size = "ndr_get_array_size($ndr, " . get_pointer_to($var_name) . ")"; > } elsif ($l->{IS_ZERO_TERMINATED} and $l->{SIZE_IS} == 0 and $l->{LENGTH_IS} == 0) { # Noheader arrays >- $length = $size = "ndr_get_string_size($ndr, sizeof(*$var_name))"; >+ $size = "ndr_get_string_size($ndr, sizeof(*$var_name))"; > } else { >- $length = $size = ParseExprExt($l->{SIZE_IS}, $env, $e->{ORIGINAL}, >+ $size = ParseExprExt($l->{SIZE_IS}, $env, $e->{ORIGINAL}, > check_null_pointer($e, $env, sub { $self->pidl(shift); }, > "return ndr_pull_error($ndr, NDR_ERR_INVALID_POINTER, \"NULL Pointer for size_is()\");"), > check_fully_dereferenced($e, $env)); > } > >+ my $array_size = $size; >+ >+ return $array_size; >+} >+ >+##################################################################### >+# parse an array - pull side >+sub ParseArrayPullGetLength($$$$$$;$) >+{ >+ my ($self,$e,$l,$ndr,$var_name,$env,$array_size) = @_; >+ >+ if (not defined($array_size)) { >+ $array_size = $self->ParseArrayPullGetSize($e, $l, $ndr, $var_name, $env); >+ } >+ >+ my $array_length = $array_size; >+ if ($l->{IS_VARYING}) { >+ my $length = "ndr_get_array_length($ndr, " . get_pointer_to($var_name) .")"; >+ $array_length = $length; >+ } >+ >+ return $array_length; >+} >+ >+##################################################################### >+# parse an array - pull side >+sub ParseArrayPullHeader($$$$$$) >+{ >+ my ($self,$e,$l,$ndr,$var_name,$env) = @_; >+ > if ((!$l->{IS_SURROUNDING}) and $l->{IS_CONFORMANT}) { > $self->pidl("NDR_CHECK(ndr_pull_array_size($ndr, " . get_pointer_to($var_name) . "));"); > } > > if ($l->{IS_VARYING}) { > $self->pidl("NDR_CHECK(ndr_pull_array_length($ndr, " . get_pointer_to($var_name) . "));"); >- $length = "ndr_get_array_length($ndr, " . get_pointer_to($var_name) .")"; > } > >- if ($length ne $size) { >- $self->pidl("if ($length > $size) {"); >+ my $array_size = $self->ParseArrayPullGetSize($e, $l, $ndr, $var_name, $env); >+ my $array_length = $self->ParseArrayPullGetLength($e, $l, $ndr, $var_name, $env, $array_size); >+ >+ if ($array_length ne $array_size) { >+ $self->pidl("if ($array_length > $array_size) {"); > $self->indent; >- $self->pidl("return ndr_pull_error($ndr, NDR_ERR_ARRAY_SIZE, \"Bad array size %u should exceed array length %u\", $size, $length);"); >+ $self->pidl("return ndr_pull_error($ndr, NDR_ERR_ARRAY_SIZE, \"Bad array size %u should exceed array length %u\", $array_size, $array_length);"); > $self->deindent; > $self->pidl("}"); > } >@@ -383,10 +412,10 @@ sub ParseArrayPullHeader($$$$$$) > } > > if (ArrayDynamicallyAllocated($e,$l) and not is_charset_array($e,$l)) { >- $self->AllocateArrayLevel($e,$l,$ndr,$var_name,$size); >+ $self->AllocateArrayLevel($e,$l,$ndr,$var_name,$array_size); > } > >- return $length; >+ return $array_length; > } > > sub compression_alg($$) >-- >1.7.4.1 > > >From bbb6dd54a1921d90dbe4b27e5beb0c97fd2e75ed Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 15 Mar 2012 13:07:47 +0100 >Subject: [PATCH 4/8] pidl/NDR/Parser: use ParseArrayPullGetLength() to get the number of array elements > >metze >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 6 +----- > 1 files changed, 1 insertions(+), 5 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index eaf673b..fe93ae1 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -1157,14 +1157,10 @@ sub ParseElementPullLevel > } > } elsif ($l->{TYPE} eq "ARRAY" and > not has_fast_array($e,$l) and not is_charset_array($e, $l)) { >- my $length = ParseExpr($l->{LENGTH_IS}, $env, $e->{ORIGINAL}); >+ my $length = $self->ParseArrayPullGetLength($e, $l, $ndr, $var_name, $env); > my $counter = "cntr_$e->{NAME}_$l->{LEVEL_INDEX}"; > my $array_name = $var_name; > >- if ($l->{IS_VARYING}) { >- $length = "ndr_get_array_length($ndr, " . get_pointer_to($var_name) .")"; >- } >- > if (my $range = has_property($e, "range")) { > my ($low, $high) = split(/,/, $range, 2); > if ($low < 0) { >-- >1.7.4.1 > > >From b371e08bcf7a067dd46723cb024267fc9ecf29c4 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 15 Mar 2012 15:07:08 +0100 >Subject: [PATCH 5/8] pidl/NDR/Parser: remember if we already know the array length > >metze >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index fe93ae1..ebdc918 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -1063,6 +1063,7 @@ sub ParseElementPullLevel > my($self,$e,$l,$ndr,$var_name,$env,$primitives,$deferred) = @_; > > my $ndr_flags = CalcNdrFlags($l, $primitives, $deferred); >+ my $array_length = undef; > > if ($l->{TYPE} eq "ARRAY" and ($l->{IS_VARYING} or $l->{IS_CONFORMANT})) { > $var_name = get_pointer_to($var_name); >@@ -1076,6 +1077,7 @@ sub ParseElementPullLevel > $self->ParseSubcontextPullEnd($e, $l, $ndr, $env); > } elsif ($l->{TYPE} eq "ARRAY") { > my $length = $self->ParseArrayPullHeader($e, $l, $ndr, $var_name, $env); >+ $array_length = $length; > > if (my $range = has_property($e, "range")) { > my ($low, $high) = split(/,/, $range, 2); >@@ -1157,10 +1159,14 @@ sub ParseElementPullLevel > } > } elsif ($l->{TYPE} eq "ARRAY" and > not has_fast_array($e,$l) and not is_charset_array($e, $l)) { >- my $length = $self->ParseArrayPullGetLength($e, $l, $ndr, $var_name, $env); >+ my $length = $array_length; > my $counter = "cntr_$e->{NAME}_$l->{LEVEL_INDEX}"; > my $array_name = $var_name; > >+ if (not defined($length)) { >+ $length = $self->ParseArrayPullGetLength($e, $l, $ndr, $var_name, $env); >+ } >+ > if (my $range = has_property($e, "range")) { > my ($low, $high) = split(/,/, $range, 2); > if ($low < 0) { >-- >1.7.4.1 > > >From 6f29762a570e62795775e12ce49dce4926913800 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 15 Mar 2012 13:13:20 +0100 >Subject: [PATCH 6/8] pidl/NDR/Parser: use helper variables for array size and length > >metze >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 22 +++++++++++++++------- > 1 files changed, 15 insertions(+), 7 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index ebdc918..3dfb521 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -338,7 +338,8 @@ sub ParseArrayPullGetSize($$$$$$) > check_fully_dereferenced($e, $env)); > } > >- my $array_size = $size; >+ $self->pidl("size_$e->{NAME}_$l->{LEVEL_INDEX} = $size;"); >+ my $array_size = "size_$e->{NAME}_$l->{LEVEL_INDEX}"; > > return $array_size; > } >@@ -356,7 +357,8 @@ sub ParseArrayPullGetLength($$$$$$;$) > my $array_length = $array_size; > if ($l->{IS_VARYING}) { > my $length = "ndr_get_array_length($ndr, " . get_pointer_to($var_name) .")"; >- $array_length = $length; >+ $self->pidl("length_$e->{NAME}_$l->{LEVEL_INDEX} = $length;"); >+ $array_length = "length_$e->{NAME}_$l->{LEVEL_INDEX}"; > } > > return $array_length; >@@ -1609,12 +1611,18 @@ sub DeclarePtrVariables($$) > } > } > >-sub DeclareArrayVariables($$) >+sub DeclareArrayVariables($$;$) > { >- my ($self,$e) = @_; >+ my ($self,$e,$pull) = @_; > > foreach my $l (@{$e->{LEVELS}}) { > next if ($l->{TYPE} ne "ARRAY"); >+ if (defined($pull)) { >+ $self->pidl("uint32_t size_$e->{NAME}_$l->{LEVEL_INDEX} = 0;"); >+ if ($l->{IS_VARYING}) { >+ $self->pidl("uint32_t length_$e->{NAME}_$l->{LEVEL_INDEX} = 0;"); >+ } >+ } > next if has_fast_array($e,$l); > next if is_charset_array($e,$l); > $self->pidl("uint32_t cntr_$e->{NAME}_$l->{LEVEL_INDEX};"); >@@ -1704,7 +1712,7 @@ sub ParseStructPull($$$$) > # declare any internal pointers we need > foreach my $e (@{$struct->{ELEMENTS}}) { > $self->DeclarePtrVariables($e); >- $self->DeclareArrayVariables($e); >+ $self->DeclareArrayVariables($e, "pull"); > $self->DeclareMemCtxVariables($e); > } > >@@ -2046,7 +2054,7 @@ sub ParseUnionPull($$$$) > next if ($double_cases{"$el->{NAME}"}); > $self->DeclareMemCtxVariables($el); > $self->DeclarePtrVariables($el); >- $self->DeclareArrayVariables($el); >+ $self->DeclareArrayVariables($el, "pull"); > $double_cases{"$el->{NAME}"} = 1; > } > >@@ -2415,7 +2423,7 @@ sub ParseFunctionPull($$) > # declare any internal pointers we need > foreach my $e (@{$fn->{ELEMENTS}}) { > $self->DeclarePtrVariables($e); >- $self->DeclareArrayVariables($e); >+ $self->DeclareArrayVariables($e, "pull"); > } > > my %double_cases = (); >-- >1.7.4.1 > > >From 17668be50b7f9461f1dd5cc008204e8ed19b1c8a Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 15 Mar 2012 13:14:48 +0100 >Subject: [PATCH 7/8] pidl/NDR/Parser: do array range validation in ParseArrayPullGetLength() > >metze >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 42 ++++++++++-------------------- > 1 files changed, 14 insertions(+), 28 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index 3dfb521..541e529 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -361,6 +361,20 @@ sub ParseArrayPullGetLength($$$$$$;$) > $array_length = "length_$e->{NAME}_$l->{LEVEL_INDEX}"; > } > >+ if (my $range = has_property($e, "range")) { >+ my ($low, $high) = split(/,/, $range, 2); >+ if ($low < 0) { >+ warning(0, "$low is invalid for the range of an array size"); >+ } >+ if ($low == 0) { >+ $self->pidl("if ($array_length > $high) {"); >+ } else { >+ $self->pidl("if ($array_length < $low || $array_length > $high) {"); >+ } >+ $self->pidl("\treturn ndr_pull_error($ndr, NDR_ERR_RANGE, \"value out of range\");"); >+ $self->pidl("}"); >+ } >+ > return $array_length; > } > >@@ -1081,20 +1095,6 @@ sub ParseElementPullLevel > my $length = $self->ParseArrayPullHeader($e, $l, $ndr, $var_name, $env); > $array_length = $length; > >- if (my $range = has_property($e, "range")) { >- my ($low, $high) = split(/,/, $range, 2); >- if ($low < 0) { >- warning(0, "$low is invalid for the range of an array size"); >- } >- if ($low == 0) { >- $self->pidl("if ($length > $high) {"); >- } else { >- $self->pidl("if ($length < $low || $length > $high) {"); >- } >- $self->pidl("\treturn ndr_pull_error($ndr, NDR_ERR_RANGE, \"value out of range\");"); >- $self->pidl("}"); >- } >- > my $nl = GetNextLevel($e, $l); > > if (is_charset_array($e,$l)) { >@@ -1169,20 +1169,6 @@ sub ParseElementPullLevel > $length = $self->ParseArrayPullGetLength($e, $l, $ndr, $var_name, $env); > } > >- if (my $range = has_property($e, "range")) { >- my ($low, $high) = split(/,/, $range, 2); >- if ($low < 0) { >- warning(0, "$low is invalid for the range of an array size"); >- } >- if ($low == 0) { >- $self->pidl("if ($length > $high) {"); >- } else { >- $self->pidl("if ($length < $low || $length > $high) {"); >- } >- $self->pidl("\treturn ndr_pull_error($ndr, NDR_ERR_RANGE, \"value out of range\");"); >- $self->pidl("}"); >- } >- > $var_name = get_array_element($var_name, $counter); > > $self->ParseMemCtxPullStart($e, $l, $ndr, $array_name); >-- >1.7.4.1 > > >From 8de071003a13fd2fbeb09c2287327886b7bff19d Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 15 Mar 2012 17:03:05 +0100 >Subject: [PATCH 8/8] pidl/NDR/Parser: also do range checks on the array size > >metze >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 25 ++++++++++++++++++++----- > 1 files changed, 20 insertions(+), 5 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index 541e529..8eb935b 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -341,6 +341,20 @@ sub ParseArrayPullGetSize($$$$$$) > $self->pidl("size_$e->{NAME}_$l->{LEVEL_INDEX} = $size;"); > my $array_size = "size_$e->{NAME}_$l->{LEVEL_INDEX}"; > >+ if (my $range = has_property($e, "range")) { >+ my ($low, $high) = split(/,/, $range, 2); >+ if ($low < 0) { >+ warning(0, "$low is invalid for the range of an array size"); >+ } >+ if ($low == 0) { >+ $self->pidl("if ($array_size > $high) {"); >+ } else { >+ $self->pidl("if ($array_size < $low || $array_size > $high) {"); >+ } >+ $self->pidl("\treturn ndr_pull_error($ndr, NDR_ERR_RANGE, \"value out of range\");"); >+ $self->pidl("}"); >+ } >+ > return $array_size; > } > >@@ -354,13 +368,14 @@ sub ParseArrayPullGetLength($$$$$$;$) > $array_size = $self->ParseArrayPullGetSize($e, $l, $ndr, $var_name, $env); > } > >- my $array_length = $array_size; >- if ($l->{IS_VARYING}) { >- my $length = "ndr_get_array_length($ndr, " . get_pointer_to($var_name) .")"; >- $self->pidl("length_$e->{NAME}_$l->{LEVEL_INDEX} = $length;"); >- $array_length = "length_$e->{NAME}_$l->{LEVEL_INDEX}"; >+ if (not $l->{IS_VARYING}) { >+ return $array_size; > } > >+ my $length = "ndr_get_array_length($ndr, " . get_pointer_to($var_name) .")"; >+ $self->pidl("length_$e->{NAME}_$l->{LEVEL_INDEX} = $length;"); >+ my $array_length = "length_$e->{NAME}_$l->{LEVEL_INDEX}"; >+ > if (my $range = has_property($e, "range")) { > my ($low, $high) = split(/,/, $range, 2); > if ($low < 0) { >-- >1.7.4.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 8815
:
7389
|
7390
|
7391
|
7392
|
7393
|
7398
|
7399
|
7400
|
7418
|
7419
|
7420
|
7421
|
7426
|
7427
|
7428
|
7429
|
7430
|
7431
|
7432
|
7433
|
7436
|
7437