SMB ioctl fixes incomplete
The recent fixes to the SMB ioctl interface seem to have resolved the issue of the copyin() that is directly part of smb_drv_ioctl().
However, this is not the only length field prefixing variable-length data in the interface. For example, smb_svcenum_t includes a member se_buflen that contains the length of the buffer for enumeration data. It also includes se_nlimit and friends which are used to control how much to write out. These are eventually used in smb_user_enum_private(), at the bottom of a long stack of calls.
smb_svcenum_t is not the only one of the union members that's variable-length, either! And any one of them that's unchecked will lead us to write over the end of the buffer. To truly fix this bug, all of these need to be carefully inspected and length checking against the original allocation length needs to be added at each stage. We cannot trust counts from userland like se_nlimit unless they are checked!
Updated by Electric Monk about 3 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
commit 1d443a93389b4537c420db30646aa0fc4c56273c Author: Dan McDonald <firstname.lastname@example.org> Date: 2017-12-22T21:21:49.000Z 8935 SMB ioctl fixes incomplete Reviewed by: Alex Wilson <email@example.com> Reviewed by: Jerry Jelinek <firstname.lastname@example.org> Reviewed by: Rui Loura <email@example.com> Reviewed by: Garrett D'Amore <firstname.lastname@example.org> Reviewed by: Dominik Hassler <email@example.com> Approved by: Garrett D'Amore <firstname.lastname@example.org>