https://bugzilla.samba.org/show_bug.cgi?id=12568
Bug ID: 12568 Summary: Integer overflow still affects xattrs.c Product: rsync Version: 3.1.2 Hardware: All OS: All Status: NEW Severity: critical Priority: P5 Component: core Assignee: way...@samba.org Reporter: shqk...@gmail.com QA Contact: rsync...@samba.org A suspicious integer overflow is found in xattrs.c:692. The code snippet is as follows. 684 for (num = 1; num <= count; num++) { 685 char *ptr, *name; 686 rsync_xa *rxa; 687 size_t name_len = read_varint(f); 688 size_t datum_len = read_varint(f); 689 size_t dget_len = datum_len > MAX_FULL_DATUM ? 1 + MAX_DIGEST_LEN : datum_len; 690 size_t extra_len = MIGHT_NEED_RPRE ? RPRE_LEN : 0; 691 if ((dget_len + extra_len < dget_len) 692 || (dget_len + extra_len + name_len < dget_len)) 693 overflow_exit("receive_xattr"); 694 ptr = new_array(char, dget_len + extra_len + name_len); 695 if (!ptr) 696 out_of_memory("receive_xattr"); 697 name = ptr + dget_len + extra_len; 698 read_buf(f, name, name_len); >From the code we can see that the security checks at line 691 and line 692 aim to filter integer overflows. Specifically, a handler, i.e. function "overflow_exit" will be invoked if the first addition "dget_len + extra_len" overflows (protected by the check at line 691) or the second addition "dget_len + extra_len + name_len" overflows (protected by the check at line 692). Here, we want to say that the later check at line 692 is insufficient to catch integer overflow. That means, there exist some integer overflows, which can bypass the later check. Assume that it's on a 32-bit machine, and "dget_len" is 100, "extra_len" is also 100, whereas "name_len" takes a very big integer value, e.g., 0xffff ffff. Hence, "dget_len + extra_len + name_len" overflows to 199, which is bigger than "dget_len", i.e. 100. As a result, an integer overflow indeed happens here, however, the overflow check at line 692 doesn't catch it. Furthermore, buffer overflow would occur at line 698. One possible workaround is to use a much stricter overflow check at line 692, as below: "dget_len + extra_len + name_len < dget_len + extra_len". Thanks. -- You are receiving this mail because: You are the QA Contact for the bug. -- Please use reply-all for most replies to avoid omitting the mailing list. To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html