The Samba-Bugzilla – Attachment 5889 Details for
Bug 7607
Buffer over-read in pidl generated client code
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for v3-5
tmp.diff (text/plain), 13.79 KB, created by
Stefan Metzmacher
on 2010-08-09 04:37:16 UTC
(
hide
)
Description:
Patch for v3-5
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2010-08-09 04:37:16 UTC
Size:
13.79 KB
patch
obsolete
>From f53e25d95ae6a8fc6e25f88b2bf354d5f43c200b Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 5 Aug 2010 10:04:57 +0200 >Subject: [PATCH 1/2] pidl: Samba3/ClientNDR - Correctly copy arrays, if r.out.size < r.in.size. > >metze > >Signed-off-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit 33d1879d5b50e2d98c1bb13b835e7cfb178e3336) >(cherry picked from commit d1e92cd2944983ecabd0511ff7c8221c1033a3a8) >Fixes bug #7607. >--- > pidl/lib/Parse/Pidl/Samba3/ClientNDR.pm | 56 ++++++++++++++++++++++++++----- > 1 files changed, 47 insertions(+), 9 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba3/ClientNDR.pm b/pidl/lib/Parse/Pidl/Samba3/ClientNDR.pm >index 68579d2..1738424 100644 >--- a/pidl/lib/Parse/Pidl/Samba3/ClientNDR.pm >+++ b/pidl/lib/Parse/Pidl/Samba3/ClientNDR.pm >@@ -15,7 +15,7 @@ use strict; > use Parse::Pidl qw(fatal warning error); > use Parse::Pidl::Util qw(has_property ParseExpr); > use Parse::Pidl::Samba4 qw(DeclLong); >-use Parse::Pidl::Samba4::Header qw(GenerateFunctionInEnv); >+use Parse::Pidl::Samba4::Header qw(GenerateFunctionInEnv GenerateFunctionOutEnv); > > use vars qw($VERSION); > $VERSION = '0.01'; >@@ -71,12 +71,27 @@ sub HeaderProperties($$) > } > } > >-sub ParseOutputArgument($$$;$$) >+sub ParseInvalidResponse($$) > { >- my ($self, $fn, $e, $r, $o) = @_; >+ my ($self, $type) = @_; >+ >+ if ($type eq "sync") { >+ $self->pidl("return NT_STATUS_INVALID_NETWORK_RESPONSE;"); >+ } elsif ($type eq "async") { >+ $self->pidl("tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE);"); >+ $self->pidl("return;"); >+ } else { >+ die("ParseInvalidResponse($type)"); >+ } >+} >+ >+sub ParseOutputArgument($$$;$$$) >+{ >+ my ($self, $fn, $e, $r, $o, $invalid_response_type) = @_; > my $level = 0; > $r = "r." unless defined($r); > $o = "" unless defined($o); >+ $invalid_response_type = "sync" unless defined($invalid_response_type); > > if ($e->{LEVELS}[0]->{TYPE} ne "POINTER" and $e->{LEVELS}[0]->{TYPE} ne "ARRAY") { > $self->pidl("return NT_STATUS_NOT_SUPPORTED;"); >@@ -97,17 +112,37 @@ sub ParseOutputArgument($$$;$$) > # Since the data is being copied into a user-provided data > # structure, the user should be able to know the size beforehand > # to allocate a structure of the right size. >- my $env = GenerateFunctionInEnv($fn, $r); >+ my $in_env = GenerateFunctionInEnv($fn, $r); >+ my $out_env = GenerateFunctionOutEnv($fn, $r); > my $l = $e->{LEVELS}[$level]; > unless (defined($l->{SIZE_IS})) { >- error($e->{ORIGINAL}, "no size known for [out] array `$e->{NAME}'"); > $self->pidl('#error No size known for [out] array `$e->{NAME}'); >+ error($e->{ORIGINAL}, "no size known for [out] array `$e->{NAME}'"); > } else { >- my $size_is = ParseExpr($l->{SIZE_IS}, $env, $e->{ORIGINAL}); >+ my $in_size_is = ParseExpr($l->{SIZE_IS}, $in_env, $e->{ORIGINAL}); >+ my $out_size_is = ParseExpr($l->{SIZE_IS}, $out_env, $e->{ORIGINAL}); >+ my $out_length_is = $out_size_is; >+ if (defined($l->{LENGTH_IS})) { >+ $out_length_is = ParseExpr($l->{LENGTH_IS}, $out_env, $e->{ORIGINAL}); >+ } >+ if ($out_size_is ne $in_size_is) { >+ $self->pidl("if (($out_size_is) > ($in_size_is)) {"); >+ $self->indent; >+ $self->ParseInvalidResponse($invalid_response_type); >+ $self->deindent; >+ $self->pidl("}"); >+ } >+ if ($out_length_is ne $out_size_is) { >+ $self->pidl("if (($out_length_is) > ($out_size_is)) {"); >+ $self->indent; >+ $self->ParseInvalidResponse($invalid_response_type); >+ $self->deindent; >+ $self->pidl("}"); >+ } > if (has_property($e, "charset")) { >- $self->pidl("memcpy(discard_const_p(uint8_t *, $o$e->{NAME}), ${r}out.$e->{NAME}, ($size_is) * sizeof(*$o$e->{NAME}));"); >+ $self->pidl("memcpy(discard_const_p(uint8_t *, $o$e->{NAME}), ${r}out.$e->{NAME}, ($out_length_is) * sizeof(*$o$e->{NAME}));"); > } else { >- $self->pidl("memcpy($o$e->{NAME}, ${r}out.$e->{NAME}, ($size_is) * sizeof(*$o$e->{NAME}));"); >+ $self->pidl("memcpy($o$e->{NAME}, ${r}out.$e->{NAME}, ($out_length_is) * sizeof(*$o$e->{NAME}));"); > } > } > } else { >@@ -281,7 +316,10 @@ sub ParseFunctionAsyncDone($$$) > foreach my $e (@{$fn->{ELEMENTS}}) { > next unless (grep(/out/, @{$e->{DIRECTION}})); > >- $self->ParseOutputArgument($fn, $e, "state->tmp.", "state->orig.out."); >+ $self->ParseOutputArgument($fn, $e, >+ "state->tmp.", >+ "state->orig.out.", >+ "async"); > } > $self->pidl(""); > >-- >1.7.0.4 > > >From 3ef559febae01dde5c5653a35ad3e524284aa7bc Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 9 Aug 2010 11:26:59 +0200 >Subject: [PATCH 2/2] rerun: make samba3-idl > >metze >--- > librpc/gen_ndr/cli_epmapper.c | 22 +++++++++++++++--- > librpc/gen_ndr/cli_ntsvcs.c | 22 +++++++++++++++--- > librpc/gen_ndr/cli_winreg.c | 47 +++++++++++++++++++++++++++++++++++----- > 3 files changed, 77 insertions(+), 14 deletions(-) > >diff --git a/librpc/gen_ndr/cli_epmapper.c b/librpc/gen_ndr/cli_epmapper.c >index c83dba6..fcfefbc 100644 >--- a/librpc/gen_ndr/cli_epmapper.c >+++ b/librpc/gen_ndr/cli_epmapper.c >@@ -380,7 +380,11 @@ static void rpccli_epm_Lookup_done(struct tevent_req *subreq) > /* Copy out parameters */ > *state->orig.out.entry_handle = *state->tmp.out.entry_handle; > *state->orig.out.num_ents = *state->tmp.out.num_ents; >- memcpy(state->orig.out.entries, state->tmp.out.entries, (state->tmp.in.max_ents) * sizeof(*state->orig.out.entries)); >+ if ((*state->tmp.out.num_ents) > (state->tmp.in.max_ents)) { >+ tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); >+ return; >+ } >+ memcpy(state->orig.out.entries, state->tmp.out.entries, (*state->tmp.out.num_ents) * sizeof(*state->orig.out.entries)); > > /* Copy result */ > state->orig.out.result = state->tmp.out.result; >@@ -453,7 +457,10 @@ NTSTATUS rpccli_epm_Lookup(struct rpc_pipe_client *cli, > /* Return variables */ > *entry_handle = *r.out.entry_handle; > *num_ents = *r.out.num_ents; >- memcpy(entries, r.out.entries, (r.in.max_ents) * sizeof(*entries)); >+ if ((*r.out.num_ents) > (r.in.max_ents)) { >+ return NT_STATUS_INVALID_NETWORK_RESPONSE; >+ } >+ memcpy(entries, r.out.entries, (*r.out.num_ents) * sizeof(*entries)); > > /* Return result */ > return NT_STATUS_OK; >@@ -549,7 +556,11 @@ static void rpccli_epm_Map_done(struct tevent_req *subreq) > /* Copy out parameters */ > *state->orig.out.entry_handle = *state->tmp.out.entry_handle; > *state->orig.out.num_towers = *state->tmp.out.num_towers; >- memcpy(state->orig.out.towers, state->tmp.out.towers, (state->tmp.in.max_towers) * sizeof(*state->orig.out.towers)); >+ if ((*state->tmp.out.num_towers) > (state->tmp.in.max_towers)) { >+ tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); >+ return; >+ } >+ memcpy(state->orig.out.towers, state->tmp.out.towers, (*state->tmp.out.num_towers) * sizeof(*state->orig.out.towers)); > > /* Copy result */ > state->orig.out.result = state->tmp.out.result; >@@ -618,7 +629,10 @@ NTSTATUS rpccli_epm_Map(struct rpc_pipe_client *cli, > /* Return variables */ > *entry_handle = *r.out.entry_handle; > *num_towers = *r.out.num_towers; >- memcpy(towers, r.out.towers, (r.in.max_towers) * sizeof(*towers)); >+ if ((*r.out.num_towers) > (r.in.max_towers)) { >+ return NT_STATUS_INVALID_NETWORK_RESPONSE; >+ } >+ memcpy(towers, r.out.towers, (*r.out.num_towers) * sizeof(*towers)); > > /* Return result */ > return NT_STATUS_OK; >diff --git a/librpc/gen_ndr/cli_ntsvcs.c b/librpc/gen_ndr/cli_ntsvcs.c >index 760ce53..e3e941a 100644 >--- a/librpc/gen_ndr/cli_ntsvcs.c >+++ b/librpc/gen_ndr/cli_ntsvcs.c >@@ -1459,7 +1459,11 @@ static void rpccli_PNP_GetDeviceList_done(struct tevent_req *subreq) > } > > /* Copy out parameters */ >- memcpy(state->orig.out.buffer, state->tmp.out.buffer, (*state->tmp.in.length) * sizeof(*state->orig.out.buffer)); >+ if ((*state->tmp.out.length) > (*state->tmp.in.length)) { >+ tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); >+ return; >+ } >+ memcpy(state->orig.out.buffer, state->tmp.out.buffer, (*state->tmp.out.length) * sizeof(*state->orig.out.buffer)); > *state->orig.out.length = *state->tmp.out.length; > > /* Copy result */ >@@ -1525,7 +1529,10 @@ NTSTATUS rpccli_PNP_GetDeviceList(struct rpc_pipe_client *cli, > } > > /* Return variables */ >- memcpy(buffer, r.out.buffer, (*r.in.length) * sizeof(*buffer)); >+ if ((*r.out.length) > (*r.in.length)) { >+ return NT_STATUS_INVALID_NETWORK_RESPONSE; >+ } >+ memcpy(buffer, r.out.buffer, (*r.out.length) * sizeof(*buffer)); > *length = *r.out.length; > > /* Return result */ >@@ -1918,7 +1925,11 @@ static void rpccli_PNP_GetDeviceRegProp_done(struct tevent_req *subreq) > > /* Copy out parameters */ > *state->orig.out.reg_data_type = *state->tmp.out.reg_data_type; >- memcpy(state->orig.out.buffer, state->tmp.out.buffer, (*state->tmp.in.buffer_size) * sizeof(*state->orig.out.buffer)); >+ if ((*state->tmp.out.buffer_size) > (*state->tmp.in.buffer_size)) { >+ tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); >+ return; >+ } >+ memcpy(state->orig.out.buffer, state->tmp.out.buffer, (*state->tmp.out.buffer_size) * sizeof(*state->orig.out.buffer)); > *state->orig.out.buffer_size = *state->tmp.out.buffer_size; > *state->orig.out.needed = *state->tmp.out.needed; > >@@ -1992,7 +2003,10 @@ NTSTATUS rpccli_PNP_GetDeviceRegProp(struct rpc_pipe_client *cli, > > /* Return variables */ > *reg_data_type = *r.out.reg_data_type; >- memcpy(buffer, r.out.buffer, (*r.in.buffer_size) * sizeof(*buffer)); >+ if ((*r.out.buffer_size) > (*r.in.buffer_size)) { >+ return NT_STATUS_INVALID_NETWORK_RESPONSE; >+ } >+ memcpy(buffer, r.out.buffer, (*r.out.buffer_size) * sizeof(*buffer)); > *buffer_size = *r.out.buffer_size; > *needed = *r.out.needed; > >diff --git a/librpc/gen_ndr/cli_winreg.c b/librpc/gen_ndr/cli_winreg.c >index 1c37f51..15017d2 100644 >--- a/librpc/gen_ndr/cli_winreg.c >+++ b/librpc/gen_ndr/cli_winreg.c >@@ -1668,7 +1668,15 @@ static void rpccli_winreg_EnumValue_done(struct tevent_req *subreq) > *state->orig.out.type = *state->tmp.out.type; > } > if (state->orig.out.value && state->tmp.out.value) { >- memcpy(state->orig.out.value, state->tmp.out.value, (*state->tmp.in.size) * sizeof(*state->orig.out.value)); >+ if ((*state->tmp.out.size) > (*state->tmp.in.size)) { >+ tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); >+ return; >+ } >+ if ((*state->tmp.out.length) > (*state->tmp.out.size)) { >+ tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); >+ return; >+ } >+ memcpy(state->orig.out.value, state->tmp.out.value, (*state->tmp.out.length) * sizeof(*state->orig.out.value)); > } > if (state->orig.out.size && state->tmp.out.size) { > *state->orig.out.size = *state->tmp.out.size; >@@ -1752,7 +1760,13 @@ NTSTATUS rpccli_winreg_EnumValue(struct rpc_pipe_client *cli, > *type = *r.out.type; > } > if (value && r.out.value) { >- memcpy(value, r.out.value, (*r.in.size) * sizeof(*value)); >+ if ((*r.out.size) > (*r.in.size)) { >+ return NT_STATUS_INVALID_NETWORK_RESPONSE; >+ } >+ if ((*r.out.length) > (*r.out.size)) { >+ return NT_STATUS_INVALID_NETWORK_RESPONSE; >+ } >+ memcpy(value, r.out.value, (*r.out.length) * sizeof(*value)); > } > if (size && r.out.size) { > *size = *r.out.size; >@@ -2823,7 +2837,15 @@ static void rpccli_winreg_QueryValue_done(struct tevent_req *subreq) > *state->orig.out.type = *state->tmp.out.type; > } > if (state->orig.out.data && state->tmp.out.data) { >- memcpy(state->orig.out.data, state->tmp.out.data, (state->tmp.in.data_size?*state->tmp.in.data_size:0) * sizeof(*state->orig.out.data)); >+ if ((state->tmp.out.data_size?*state->tmp.out.data_size:0) > (state->tmp.in.data_size?*state->tmp.in.data_size:0)) { >+ tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); >+ return; >+ } >+ if ((state->tmp.out.data_length?*state->tmp.out.data_length:0) > (state->tmp.out.data_size?*state->tmp.out.data_size:0)) { >+ tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); >+ return; >+ } >+ memcpy(state->orig.out.data, state->tmp.out.data, (state->tmp.out.data_length?*state->tmp.out.data_length:0) * sizeof(*state->orig.out.data)); > } > if (state->orig.out.data_size && state->tmp.out.data_size) { > *state->orig.out.data_size = *state->tmp.out.data_size; >@@ -2904,7 +2926,13 @@ NTSTATUS rpccli_winreg_QueryValue(struct rpc_pipe_client *cli, > *type = *r.out.type; > } > if (data && r.out.data) { >- memcpy(data, r.out.data, (r.in.data_size?*r.in.data_size:0) * sizeof(*data)); >+ if ((r.out.data_size?*r.out.data_size:0) > (r.in.data_size?*r.in.data_size:0)) { >+ return NT_STATUS_INVALID_NETWORK_RESPONSE; >+ } >+ if ((r.out.data_length?*r.out.data_length:0) > (r.out.data_size?*r.out.data_size:0)) { >+ return NT_STATUS_INVALID_NETWORK_RESPONSE; >+ } >+ memcpy(data, r.out.data, (r.out.data_length?*r.out.data_length:0) * sizeof(*data)); > } > if (data_size && r.out.data_size) { > *data_size = *r.out.data_size; >@@ -4629,7 +4657,11 @@ static void rpccli_winreg_QueryMultipleValues_done(struct tevent_req *subreq) > /* Copy out parameters */ > memcpy(state->orig.out.values, state->tmp.out.values, (state->tmp.in.num_values) * sizeof(*state->orig.out.values)); > if (state->orig.out.buffer && state->tmp.out.buffer) { >- memcpy(state->orig.out.buffer, state->tmp.out.buffer, (*state->tmp.in.buffer_size) * sizeof(*state->orig.out.buffer)); >+ if ((*state->tmp.out.buffer_size) > (*state->tmp.in.buffer_size)) { >+ tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); >+ return; >+ } >+ memcpy(state->orig.out.buffer, state->tmp.out.buffer, (*state->tmp.out.buffer_size) * sizeof(*state->orig.out.buffer)); > } > *state->orig.out.buffer_size = *state->tmp.out.buffer_size; > >@@ -4701,7 +4733,10 @@ NTSTATUS rpccli_winreg_QueryMultipleValues(struct rpc_pipe_client *cli, > /* Return variables */ > memcpy(values, r.out.values, (r.in.num_values) * sizeof(*values)); > if (buffer && r.out.buffer) { >- memcpy(buffer, r.out.buffer, (*r.in.buffer_size) * sizeof(*buffer)); >+ if ((*r.out.buffer_size) > (*r.in.buffer_size)) { >+ return NT_STATUS_INVALID_NETWORK_RESPONSE; >+ } >+ memcpy(buffer, r.out.buffer, (*r.out.buffer_size) * sizeof(*buffer)); > } > *buffer_size = *r.out.buffer_size; > >-- >1.7.0.4 >
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
Flags:
gd
:
review+
Actions:
View
Attachments on
bug 7607
:
5885
| 5889 |
5890