Bug 9567 - oLschema2ldif fail to import any SYNTAX that isn't in dsdb_syntaxes array
Summary: oLschema2ldif fail to import any SYNTAX that isn't in dsdb_syntaxes array
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Tools (show other bugs)
Version: 4.10.0
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL: https://gitlab.com/samba-team/samba/m...
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-17 11:43 UTC by Alejandro Escanero Blanco
Modified: 2019-06-11 10:44 UTC (History)
4 users (show)

See Also:

Attachments
patch from hansmi (10.03 KB, patch)
2019-03-27 23:10 UTC, Andrew Bartlett 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alejandro Escanero Blanco 2013-01-17 11:43:58 UTC 
oLschema2ldif fail to import SYNTAX 1.3.6.1.4.1.1466.115.121.1.41 (PostalAddress)


Backtrace give us:

#0  0x00007ffff6a5653d in __strcasecmp_l_ssse3 () from /lib64/libc.so.6
#1  0x00007ffff710eecd in find_syntax_map_by_standard_oid (standard_oid=0x647890 "1.3.6.1.4.1.1466.115.121.1.41") at ../source4/dsdb/schema/schema_syntax.c:2656
#2  0x0000000000403244 in process_entry (mem_ctx=0x60e9b0, 
    entry=0x6464e0 "attributetype ( 1.3.6.1.4.1.7200.2.3.6 NAME 'LikePostalAddress'        DESC 'TEST'\tSYNTAX 1.3.6.1.4.1.1466.115.121.1.41 )")
    at ../source4/utils/oLschema2ldif.c:475
#3  0x00000000004034e7 in process_file (in=0x60f170, out=0x60fef0) at ../source4/utils/oLschema2ldif.c:551
#4  0x00000000004039be in main (argc=7, argv=0x7fffffffe5a8) at ../source4/utils/oLschema2ldif.c:683
Comment 1 Alejandro Escanero Blanco 2013-03-18 11:00:51 UTC 
Why don't check this block the long of dsdb_syntaxes array?
---
        for (i=0; dsdb_syntaxes[i].ldap_oid; i++) {
                 dsdb_syntaxes[i].ldap_oid));
                if (strcasecmp(standard_oid, dsdb_syntaxes[i].ldap_oid) == 0) {
                        return &dsdb_syntaxes[i];
                }
        }
---
Comment 2 Alejandro Escanero Blanco 2013-03-18 11:08:07 UTC 
Also can't import 1.3.6.1.4.1.1466.115.121.1.22 -> AD-Syntaxes.txt:Facsimile Telephone Number  1.3.6.1.4.1.1466.115.121.1.22
Comment 3 Andrew Bartlett 2019-03-27 23:09:57 UTC 
Michael Hanselmann has provided a fix for this and related issues.

I'm marking this bug as embargoed for a day to allow anyone to come up with a plausible way this could be exploited, but as I understand it we have not issued a CVE for crashes in TDB from malformed DB files, for example. 

Folks are generally trying to convert well-known schema, not random files found online without any inspection.
Comment 4 Andrew Bartlett 2019-03-27 23:10:52 UTC 
Created attachment 15013 [details]
patch from hansmi

I'll update the patch to have the right BUG: markings and review tags before pushing.
Comment 5 Stefan Metzmacher 2019-03-27 23:57:50 UTC 
Comment on attachment 15013 [details]
patch from hansmi

We loop over dsdb_syntaxes in two different ways:

for (i=0; i < ARRAY_SIZE(dsdb_syntaxes); i++) {

and

for (i=0; dsdb_syntaxes[i].ldap_oid; i++) {

Wouldn't that patch fix the 2nd one, but break the first one?
Comment 6 Stefan Metzmacher 2019-03-28 00:02:15 UTC 
(In reply to Stefan Metzmacher from comment #5)

As all elements have ldap_oid != NULL we could just always use 

for (i=0; i < ARRAY_SIZE(dsdb_syntaxes); i++) {

as a fix.

This should be its own commit, separate of the test changes.

If it works the test could be the fist commit with a selftest/knownfail.d/ entry,
which will be removed in the 2nd commit.
Comment 7 Andrew Bartlett 2019-04-01 22:59:21 UTC 
(In reply to Stefan Metzmacher from comment #6)
Thanks, I've made that alternate fix.

I've submitted a public merge request for this here:

https://gitlab.com/samba-team/samba/merge_requests/353

I would normally agree on splitting up the patch however, the test loops on the old code (so preventing a knownfail) and given the marginal importance of this whole area I can't justify fixing it up.
Comment 8 Andrew Bartlett 2019-06-11 10:44:48 UTC 
Fixed in master by 29d7c80ee4d78e1dbdd506770a9cb7b34afa0ed0 for Samba 4.11