Bug 12495 - source3/locking/leases_db.c -> leases_db_del() crashes smbd when the record does not exist in tdb
Summary: source3/locking/leases_db.c -> leases_db_del() crashes smbd when the record d...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other (show other bugs)
Version: 4.2.14
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-03 14:22 UTC by YOUZHONG YANG
Modified: 2017-01-04 13:55 UTC (History)
1 user (show)

See Also:


Attachments
patch (1.08 KB, patch)
2017-01-03 15:40 UTC, YOUZHONG YANG
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description YOUZHONG YANG 2017-01-03 14:22:29 UTC
and here is the fix we verified:

 	rec = dbwrap_fetch_locked(leases_db, talloc_tos(), db_key);
 	TALLOC_FREE(db_key.dptr);
 	if (rec == NULL) {
-		return NT_STATUS_NOT_FOUND;
+		return NT_STATUS_INTERNAL_ERROR;
 	}
 	db_value = dbwrap_record_get_value(rec);
 	if (db_value.dsize == 0) {
-		status = NT_STATUS_INTERNAL_ERROR;
-		goto out;
+		TALLOC_FREE(rec);
+		return NT_STATUS_NOT_FOUND;
 	}
 
 	value = talloc(talloc_tos(), struct leases_db_value);
Comment 1 Volker Lendecke 2017-01-03 14:41:07 UTC
Can you post that patch to samba-technical@lists.samba.org with git information? See https://wiki.samba.org/index.php/Contribute for some quick info
Comment 2 YOUZHONG YANG 2017-01-03 15:40:43 UTC
Created attachment 12790 [details]
patch
Comment 3 YOUZHONG YANG 2017-01-04 13:55:18 UTC
Sorry my bad. I was looking at 4.2.x code and didn't check master branch. The issue seems already fixed.

The crash in 4.2.x is here:

https://github.com/samba-team/samba/blob/v4-2-stable/source3/locking/leases_db.c#L311

if db_value.dsize is 0, it jumps to “out”, and tries to TALLOC_FREE(value), but value is uninitialized.