Bug 11838 - LDAP Search Attribute-"Range" Error if "Range" stars with a capital letter
LDAP Search Attribute-"Range" Error if "Range" stars with a capital letter
Product: Samba 4.1 and newer
Classification: Unclassified
x86 Linux
: P5 minor
: ---
Assigned To: Karolin Seeger
Samba QA Contact
Depends on:
  Show dependency treegraph
Reported: 2016-04-14 14:07 UTC by Timo
Modified: 2016-08-05 07:37 UTC (History)
2 users (show)

See Also:

git-am fix for master. (1.23 KB, patch)
2016-06-28 22:15 UTC, Jeremy Allison
abartlet: review-
git-am fix for master. (1.70 KB, patch)
2016-06-28 22:39 UTC, Jeremy Allison
abartlet: review+
git-am fix for 4.4.next, 4.3.next. (1.82 KB, patch)
2016-06-30 19:54 UTC, Jeremy Allison
abartlet: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Timo 2016-04-14 14:07:46 UTC
Do a LDAP-Search with attribute "Member;Range=0-*"

If you type "Range" with a capital letter you will get an error

[2016/04/14 13:48:11, 10, pid=15641, effective(0, 0), real(0, 0), class=ldb] ../lib/ldb-samba/ldb_wrap.c:72(ldb_wrap_debug)
  ldb: ldb_asprintf/set_errstring: range request error: range request malformed

If you type "range" with a lower case letter the search complets without an error.

We found the problem because we want to use SCCM with a SAMBA AD-DC. In our testlab with a Windows AD-DC we can perform both searches ("Range" and "range") without problems.

Comment 1 TJ 2016-06-27 18:59:43 UTC
I can confirm this (run into another utility which generates this kind of query).  The problem appears in source4/dsdb/samdb/ldb_modules/ranged_results.c  There is a case insensitive check for ";range=", but the subsequent sscanf calls look for lower case range only.  If it's safe to overwrite p (and that capitals aren't against spec), then maybe something along these lines as a fix (including line numbers for reference):

201         /* Strip the range request from the attribute */
202         for (i = 0; req->op.search.attrs && req->op.search.attrs[i]; i++) {
203                 char *p;
204                 new_attrs = talloc_realloc(req, new_attrs, const char *, i+2);
205                 new_attrs[i] = req->op.search.attrs[i];
206                 new_attrs[i+1] = NULL;
207                 p = strchr(new_attrs[i], ';');
208                 if (!p) {
209                         continue;
210                 }
211                 if (strncasecmp(p, ";range=", strlen(";range=")) != 0) {
212                         continue;
213                 }
214                 /* make sure ;range= is lower case. Fix for bug 41247*/
215                 strncpy(p,";range=",strlen(";range=");
216                 /* end fix */
217                 end = (unsigned int)-1;
218                 if (sscanf(p, ";range=%u-*", &start) != 1) {
219                         if (sscanf(p, ";range=%u-%u", &start, &end) != 2) {
220                                 ldb_asprintf_errstring(ldb,
221                                 "range request error: "
222                                 "range request malformed");
223                                 return LDB_ERR_UNWILLING_TO_PERFORM;
224                         }
225                 }

I'm on a fairly tight schedule to get samba and the utility working together so any feedback here would be greatly appreciated.
Comment 2 Jeremy Allison 2016-06-28 22:15:34 UTC
Created attachment 12231 [details]
git-am fix for master.

Does this work for you ? If so I'll propose for master and get back ports for 4.4.next, 4.3.next.
Comment 3 Andrew Bartlett 2016-06-28 22:23:57 UTC
Comment on attachment 12231 [details]
git-am fix for master.

I think we first need to put the attribute on the stack before we memcpy(), because we don't own the attribute.
Comment 4 Jeremy Allison 2016-06-28 22:31:19 UTC
Oh, good catch. I thought the talloc_realloc() above was doing that, but of course it's not. I withdraw that patch and I'll need to rework it.
Comment 5 Jeremy Allison 2016-06-28 22:35:19 UTC
Oh wait a minute. We've already checked case insensitively for the ";range=" string, so we really don't need it in the sscanf....

Fix to follow.
Comment 6 Jeremy Allison 2016-06-28 22:39:55 UTC
Created attachment 12232 [details]
git-am fix for master.

I think this is a correct fix - no modification of the string needed.
Comment 7 Andrew Bartlett 2016-06-28 22:43:45 UTC
Comment on attachment 12232 [details]
git-am fix for master.


Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Comment 8 Jeremy Allison 2016-06-28 22:53:57 UTC
Ok. pushing to master then I'll back-port for 4.4.next, 4.3.next.
Comment 9 Jeremy Allison 2016-06-30 19:54:00 UTC
Created attachment 12247 [details]
git-am fix for 4.4.next, 4.3.next.

Cherry-picked from master.
Comment 10 TJ 2016-07-05 14:20:09 UTC
Tested with the latest patch from Jeremy, it's working as expected now.  Thanks much :)

Comment 11 Timo 2016-07-06 10:06:50 UTC
Thank you guys!
Comment 12 Jeremy Allison 2016-07-11 21:01:05 UTC
Comment on attachment 12247 [details]
git-am fix for 4.4.next, 4.3.next.

Ping. Andrew, can you +1 this so we can get into 4.4.next please ?
Comment 13 Jeremy Allison 2016-07-12 16:07:08 UTC
Karolin please apply to 4.4.next, 4.3.next.

Thanks !
Comment 14 Karolin Seeger 2016-08-02 07:28:14 UTC
(In reply to Jeremy Allison from comment #13)
Pushed to autobuild-v4-[4|3]-test.
Comment 15 Karolin Seeger 2016-08-05 07:37:07 UTC
(In reply to Karolin Seeger from comment #14)
Pushed to both branches.
Closing out bug report.