Bug 13840 - Read out of bounds in binary registry file parsing
Read out of bounds in binary registry file parsing
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: DCE-RPCs and pipes
4.10.0rc4
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
https://gitlab.com/samba-team/samba/m...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-18 01:41 UTC by Andrew Bartlett
Modified: 2019-06-20 09:39 UTC (History)
6 users (show)

See Also:


Attachments
gitlab patch (455 bytes, patch)
2019-03-18 01:41 UTC, Andrew Bartlett
no flags Details
The Gitlab patch Andrew meant to attach (4.12 KB, patch)
2019-03-19 21:59 UTC, Douglas Bagnall
no flags Details
git-am fix for 4.10.next, 4.9.next. (18.83 KB, patch)
2019-03-22 19:03 UTC, Jeremy Allison
dbagnall: review+
abartlet: review+
Details
Follow-up fix for master (773 bytes, patch)
2019-05-16 14:08 UTC, Ralph Böhme
slow: review? (abartlet)
dbagnall: review+
Details
Fixup patch for 4.9 and 4.10 (1.14 KB, patch)
2019-06-06 08:02 UTC, Karolin Seeger
kseeger: review+
slow: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2019-03-18 01:41:25 UTC
Created attachment 14935 [details]
gitlab patch

This looks like just a crash (the memcmp() would fail), but to be sure I'm filing a bug so we can track it. 

Identified using ASAN: Supplying a malformed registry hive file to the
registry hive I/O code can lead to out-of-bounds reads.
Comment 1 Andrew Bartlett 2019-03-18 01:44:13 UTC
I think this is very close to being a security issue, but as long as it 'just' out of bounds reads I don't think we will CVE it.  But I will say both the prs_ based code here and the registry parsing stuff is very old, and if you don't find something more serious I'll be shocked. 

There is a call path via:
_winreg_RestoreKey()
reg_restorekey()
restore_registry_key()
regfio_rootkey()
next_nk_record()
Comment 2 Douglas Bagnall 2019-03-19 21:59:41 UTC
Created attachment 14951 [details]
The Gitlab patch Andrew meant to attach
Comment 3 Jeremy Allison 2019-03-22 19:03:22 UTC
Created attachment 14982 [details]
git-am fix for 4.10.next, 4.9.next.

Back-port cherry-picked from master. Applies cleanly to 4.10.next, 4.9.next.
Comment 4 Jeremy Allison 2019-03-22 19:29:25 UTC
Karolin please apply for 4.10.next, 4.9.next.
Comment 5 Karolin Seeger 2019-03-28 08:31:31 UTC
(In reply to Jeremy Allison from comment #4)
Pushed to autobuild-v4-{10,9}-test.
Comment 6 Karolin Seeger 2019-04-02 07:58:38 UTC
Pushed to v4-9-test, pushed again to autobuild-v4-10-test.
Comment 7 Karolin Seeger 2019-04-03 10:30:12 UTC
(In reply to Karolin Seeger from comment #6)
Pushed to v4-10-test.
Closing out bug report.

Thanks!
Comment 8 Ralph Böhme 2019-05-16 14:07:00 UTC
This causes a compilation error on Fedora 29:

../source3/registry/tests/test_regfio.c: In function ‘teardown_context’:
../source3/registry/tests/test_regfio.c:86:3: error: implicit declaration of function ‘unlink’; did you mean ‘unix’? [-Werror=implicit-function-declaration]
unlink(test_ctx->tmp_regfile);
^~~~~~
unix
Comment 9 Ralph Böhme 2019-05-16 14:08:13 UTC
Created attachment 15161 [details]
Follow-up fix for master
Comment 10 Karolin Seeger 2019-06-06 08:02:17 UTC
Created attachment 15219 [details]
Fixup patch for 4.9 and 4.10
Comment 11 Ralph Böhme 2019-06-06 13:10:06 UTC
Reassigning to Karolin for inclusion in 4.9 and 4.10.
Comment 12 Karolin Seeger 2019-06-06 13:42:04 UTC
(In reply to Ralph Böhme from comment #11)
Pushed follow-up patch to autobuild-v4-{9,10}-test.
Comment 13 Karolin Seeger 2019-06-20 09:39:27 UTC
(In reply to Karolin Seeger from comment #12)
Pushed to both branches.
Closing out bug report.

Thanks!