Bug 9213 - Bad ASN.1 NegTokenInit packet can cause invalid free.
Summary: Bad ASN.1 NegTokenInit packet can cause invalid free.
Alias: None
Product: Samba 3.6
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P5 regression
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
Depends on:
Reported: 2012-09-25 22:44 UTC by Jeremy Allison
Modified: 2015-07-31 15:51 UTC (History)
1 user (show)

See Also:

Patch for all releases. (978 bytes, patch)
2012-09-26 17:52 UTC, Jeremy Allison
vl: review+
Wireshark capture (4.78 KB, application/octet-stream)
2012-09-27 18:59 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2012-09-25 22:44:11 UTC
Sigh. We've had this bug before (it isn't a security hole btw.). at the SNIA plugfest Codenomicon showed a bad OID sent in a NegTokenInit can cause asn1_read_OID() to fail to read an OID string without setting asn1->has_error in the data ASN.1 structure.

The OID array in the function definition :

bool spnego_parse_negTokenInit(TALLOC_CTX *ctx,
                               DATA_BLOB blob,
                               char *OIDs[ASN1_MAX_OIDS],
                               char **principal,
                               DATA_BLOB *secblob)

is a BAD interface. And we should always set OIDs[0...ASN1_MAX_OIDS-1] = NULL
before attempting to do any parsing, in the same way we set *principal = NULL and *secblob = data_blob_null.

Patches to follow. More when I have coordinated with Codenomicon to get a packet trace of the exact packet that caused the problem.

Comment 1 Jeremy Allison 2012-09-26 17:52:57 UTC
Created attachment 7943 [details]
Patch for all releases.

As previously mentioned, this isn't the completely correct patch but will fix this generic uninitialized free error once and for all, and once I have the packet trace from Codenomicon I will fix the underlying issue.

This patch applies to all released versions - 4.0.0rc, 3.6.next and 3.5.next.

Comment 2 Jeremy Allison 2012-09-26 22:29:50 UTC
Re-assigning to Karolin for inclusion in:

Comment 3 Jeremy Allison 2012-09-27 18:59:06 UTC
Created attachment 7955 [details]
Wireshark capture

Packet number 9 is the offending one.
Comment 4 Karolin Seeger 2012-09-28 07:18:53 UTC
(In reply to comment #2)
> Re-assigning to Karolin for inclusion in:
> 3.5.next
> 3.6.next
> 4.0.0rc.next.

Pushed to autobuild-v4-0-test, v3-6-test and v3-5-test.
Re-assigning to Jeremy.
Comment 5 Stefan Metzmacher 2015-06-15 07:47:11 UTC
(In reply to Karolin Seeger from comment #4)

Karolin assigned to me instead of Jeremy...

What's left to do here?
Comment 6 Stefan Metzmacher 2015-07-31 08:45:51 UTC
(In reply to Stefan (metze) Metzmacher from comment #5)

Jeremy, can we close this?
Comment 7 Jeremy Allison 2015-07-31 15:51:19 UTC
Yes, the complete fix went in (reviewed by Ronnie) some time ago.