Skip to content

Commit 3df690b

Browse files
gregkhsmfrench
authored andcommitted
smb: client: fix OOB reads parsing symlink error response
When a CREATE returns STATUS_STOPPED_ON_SYMLINK, smb2_check_message() returns success without any length validation, leaving the symlink parsers as the only defense against an untrusted server. symlink_data() walks SMB 3.1.1 error contexts with the loop test "p < end", but reads p->ErrorId at offset 4 and p->ErrorDataLength at offset 0. When the server-controlled ErrorDataLength advances p to within 1-7 bytes of end, the next iteration will read past it. When the matching context is found, sym->SymLinkErrorTag is read at offset 4 from p->ErrorContextData with no check that the symlink header itself fits. smb2_parse_symlink_response() then bounds-checks the substitute name using SMB2_SYMLINK_STRUCT_SIZE as the offset of PathBuffer from iov_base. That value is computed as sizeof(smb2_err_rsp) + sizeof(smb2_symlink_err_rsp), which is correct only when ErrorContextCount == 0. With at least one error context the symlink data sits 8 bytes deeper, and each skipped non-matching context shifts it further by 8 + ALIGN(ErrorDataLength, 8). The check is too short, allowing the substitute name read to run past iov_len. The out-of-bound heap bytes are UTF-16-decoded into the symlink target and returned to userspace via readlink(2). Fix this all up by making the loops test require the full context header to fit, rejecting sym if its header runs past end, and bound the substitute name against the actual position of sym->PathBuffer rather than a fixed offset. Because sub_offs and sub_len are 16bits, the pointer math will not overflow here with the new greater-than. Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Shyam Prasad N <sprasad@microsoft.com> Cc: Tom Talpey <tom@talpey.com> Cc: Bharath SM <bharathsm@microsoft.com> Cc: linux-cifs@vger.kernel.org Cc: samba-technical@lists.samba.org Cc: stable <stable@kernel.org> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Assisted-by: gregkh_clanker_t1000 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent 3d8b9d0 commit 3df690b

1 file changed

Lines changed: 12 additions & 8 deletions

File tree

fs/smb/client/smb2file.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,11 @@ static struct smb2_symlink_err_rsp *symlink_data(const struct kvec *iov)
2727
{
2828
struct smb2_err_rsp *err = iov->iov_base;
2929
struct smb2_symlink_err_rsp *sym = ERR_PTR(-EINVAL);
30+
u8 *end = (u8 *)err + iov->iov_len;
3031
u32 len;
3132

3233
if (err->ErrorContextCount) {
33-
struct smb2_error_context_rsp *p, *end;
34+
struct smb2_error_context_rsp *p;
3435

3536
len = (u32)err->ErrorContextCount * (offsetof(struct smb2_error_context_rsp,
3637
ErrorContextData) +
@@ -39,8 +40,7 @@ static struct smb2_symlink_err_rsp *symlink_data(const struct kvec *iov)
3940
return ERR_PTR(-EINVAL);
4041

4142
p = (struct smb2_error_context_rsp *)err->ErrorData;
42-
end = (struct smb2_error_context_rsp *)((u8 *)err + iov->iov_len);
43-
do {
43+
while ((u8 *)p + sizeof(*p) <= end) {
4444
if (le32_to_cpu(p->ErrorId) == SMB2_ERROR_ID_DEFAULT) {
4545
sym = (struct smb2_symlink_err_rsp *)p->ErrorContextData;
4646
break;
@@ -50,14 +50,16 @@ static struct smb2_symlink_err_rsp *symlink_data(const struct kvec *iov)
5050

5151
len = ALIGN(le32_to_cpu(p->ErrorDataLength), 8);
5252
p = (struct smb2_error_context_rsp *)(p->ErrorContextData + len);
53-
} while (p < end);
53+
}
5454
} else if (le32_to_cpu(err->ByteCount) >= sizeof(*sym) &&
5555
iov->iov_len >= SMB2_SYMLINK_STRUCT_SIZE) {
5656
sym = (struct smb2_symlink_err_rsp *)err->ErrorData;
5757
}
5858

59-
if (!IS_ERR(sym) && (le32_to_cpu(sym->SymLinkErrorTag) != SYMLINK_ERROR_TAG ||
60-
le32_to_cpu(sym->ReparseTag) != IO_REPARSE_TAG_SYMLINK))
59+
if (!IS_ERR(sym) &&
60+
((u8 *)sym + sizeof(*sym) > end ||
61+
le32_to_cpu(sym->SymLinkErrorTag) != SYMLINK_ERROR_TAG ||
62+
le32_to_cpu(sym->ReparseTag) != IO_REPARSE_TAG_SYMLINK))
6163
sym = ERR_PTR(-EINVAL);
6264

6365
return sym;
@@ -128,8 +130,10 @@ int smb2_parse_symlink_response(struct cifs_sb_info *cifs_sb, const struct kvec
128130
print_len = le16_to_cpu(sym->PrintNameLength);
129131
print_offs = le16_to_cpu(sym->PrintNameOffset);
130132

131-
if (iov->iov_len < SMB2_SYMLINK_STRUCT_SIZE + sub_offs + sub_len ||
132-
iov->iov_len < SMB2_SYMLINK_STRUCT_SIZE + print_offs + print_len)
133+
if ((char *)sym->PathBuffer + sub_offs + sub_len >
134+
(char *)iov->iov_base + iov->iov_len ||
135+
(char *)sym->PathBuffer + print_offs + print_len >
136+
(char *)iov->iov_base + iov->iov_len)
133137
return -EINVAL;
134138

135139
return smb2_parse_native_symlink(path,

0 commit comments

Comments
 (0)