[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.
> 
> 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.

How does that help, if name could be:

foo",cn="bar

My point is that by saying 'this is the callers problem', you multiply
the number of places that must care about the subtle nature of how LDB
will parse the constructed string.  

We are loosing information in this API:  The caller knows that these are
individual data items, not to be combined.  LDB knows about each element
in the dn as a individual datum.  However, the printf API causes these
to be merged in a way that looses important information!

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/20061227/2a6295e2/attachment.bin