[LDB] was ldb_dn_build_child safer than ldb_dn_add_child_fmt?

[Andrew Bartlett]

On Wed, 2006-12-27 at 10:25 +0000, idra at samba.org wrote:
> On Wed, Dec 27, 2006 at 02:27:15PM +1100, Andrew Bartlett wrote:
> > I dug up some old code recently, and came across one puzzling part of
> > the change from ldb_dn_build_child(ctx, attribute, value, dn) to
> > ldb_dn_add_child_fmt(dn, fmt, ...).
> > 
> > Almost all the callers of ldb_dn_add_child_fmt() simply wish to add
> > exactly one element to the DN.  
> > 
> > Eg:  
> > 
> > ./rpc_server/lsa/dcesrv_lsa.c:836:      
> > if ( ! ldb_dn_add_child_fmt(msg->dn, "cn=%s", name)) {
> > 
> > name in this case is the name of a newly trusted domain.  
> > 
> > In these cases, I think this API is over-flexible, in what it could
> > allow an attacker.  The caller expects name to be
> > 
> > foo
> > 
> > But imagine name was to become:
> > 
> > foo,cn=users
> > 
> > The resultant DN would be cn=foo,cn=users,<base>.  My claim is that this
> > extra DN component could allow an attacker to play games, etc...
> > 
> > I was wondering if it would be worth adding the old API
> > (ldb_dn_build_child) back, to ensure that these elements cannot be
> > misinterpreted?
> > 
> > Thoughts?
> 
> This is an internal API, there is no reason I can see we should care
> about this kind of problems at this level. If you blindly accept user
> input without validation from the caller, then there, in the caller, is
> the problem.

The problem is, where should I validate the input?  In every possible
function that will deal with creating a DN?

We have seen what this does, and it is the world of pain that is SQL
insertion attacks.  

> If you expect the name to be a single elment you can change
> it this way:
> if ( ! ldb_dn_add_child_fmt(msg->dn, "cn=\"%s\"", name)) {
> using quotes. but I would rather do some more checks in the caller.
> 
> In any case killing the old function was one of my prioirties as it was
> too ugly to survive under many point of views.

It also kept separated data separate.  In the old system, only one
function was responsible for creating an escaped DN, and that function
could do it right.  Now we have every caller having to correctly create
escaped DN components, and I doubt we will get every case right!

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Red Hat Inc.                  http://redhat.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20061229/5cc32797/attachment.bin