| Bug #52748 | Semi-Sync ACK packet isn't check for length | ||
|---|---|---|---|
| Submitted: | 11 Apr 2010 19:43 | Modified: | 20 Jul 2010 14:13 |
| Reporter: | Jan Kneschke | Email Updates: | |
| Status: | Closed | Impact on me: | |
| Category: | MySQL Server: Replication | Severity: | S1 (Critical) |
| Version: | 5.5.2 | OS: | Any |
| Assigned to: | Zhenxing He | CPU Architecture: | Any |
[12 Apr 2010 9:00]
Sveta Smirnova
Thank you for the report. > Send a binlog filename in the ACK packet that is > FN_LENGTH: How can I do this? MySQL server refuses to start with such file name for binary log.
[12 Apr 2010 13:54]
Jan Kneschke
I used the modified version of MySQL Proxy's replicant plugin to trigger it, but you can also just change plugins/semisync/semisync_slave.cc:
int ReplSemiSyncSlave::slaveReply(...) {
...
uchar reply_buffer[REPLY_MAGIC_NUM_LEN
+ REPLY_BINLOG_POS_LEN
+ REPLY_BINLOG_NAME_LEN + 255]; /* <--- to-crash: get some more room */
int reply_res, name_len = strlen(binlog_filename);
function_enter(kWho);
/* Prepare the buffer of the reply. */
reply_buffer[REPLY_MAGIC_NUM_OFFSET] = kPacketMagicNum;
int8store(reply_buffer + REPLY_BINLOG_POS_OFFSET, binlog_filepos);
memcpy(reply_buffer + REPLY_BINLOG_NAME_OFFSET,
binlog_filename,
name_len); /** to-crash: no trailing 0 */
/* to-crash: and now append a 255 char long string to make it over run the static buffer on the master */
memset(reply_buffer + EPLY_BINLOG_NAME_OFFSET + name_len, 'A', 255);
[13 Apr 2010 19:43]
Sveta Smirnova
Thank you for the feedback. I still can not reproduce it in mysql-next-mr tree (5.6.99). Could you please try this tree in your environment and inform us if problem still exists?
[5 May 2010 7:19]
Zhenxing He
will check the length and use strncpy to make the code safer.
[24 May 2010 2:53]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/108969 3057 He Zhenxing 2010-05-24 BUG#52748 Semi-Sync ACK packet isn't check for length Check the length and use strncpy to make the code safer.
[24 May 2010 2:57]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/108970 3057 He Zhenxing 2010-05-24 BUG#52748 Semi-Sync ACK packet isn't check for length Check the length and use strncpy to make the code safer.
[31 May 2010 8:59]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/109588 3057 He Zhenxing 2010-05-31 BUG#52748 Semi-Sync ACK packet isn't check for length Check the length and use strncpy to make the code safer.
[1 Jun 2010 8:54]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/109659 3132 He Zhenxing 2010-06-01 BUG#52748 Semi-Sync ACK packet isn't check for length Check the length and use strncpy to make the code safer. @ plugin/semisync/semisync_master.cc replace strcpy with strncpy to make the code safer
[1 Jun 2010 9:00]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/109661 3077 He Zhenxing 2010-06-01 [merge] BUG#52748 Semi-Sync ACK packet isn't check for length Check the length and use strncpy to make the code safer.
[1 Jun 2010 9:15]
Zhenxing He
pushed to 5.1-rep-semisync and trunk-bugfixing
[1 Jun 2010 10:24]
Zhenxing He
pushed to next-mr-bugfixing
[2 Jun 2010 10:57]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/109943 3133 He Zhenxing 2010-06-02 Post fix for bug#52748
[15 Jun 2010 8:16]
Bugs System
Pushed into 5.5.5-m3 (revid:alik@sun.com-20100615080459-smuswd9ooeywcxuc) (version source revid:marko.makela@oracle.com-20100601134335-ccthwwru23kn09qw) (merge vers: 5.1.48) (pib:16)
[15 Jun 2010 8:33]
Bugs System
Pushed into mysql-next-mr (revid:alik@sun.com-20100615080558-cw01bzdqr1bdmmec) (version source revid:marko.makela@oracle.com-20100601134335-ccthwwru23kn09qw) (pib:16)
[20 Jul 2010 14:13]
Paul DuBois
Noted in 5.5.5 changelog. ACK packets in semisynchronous replication were not checked for length and malformed packets could cause a server crash.
Description: Sending a crafted ACK packet in a semi-sync replication triggers a segfault. int ReplSemiSyncMaster::readSlaveReply(NET *net, uint32 server_id, const char *event_buf) { ... char log_file_name[FN_REFLEN]; ... strcpy(log_file_name, (const char*)packet + REPLY_BINLOG_NAME_OFFSET); How to repeat: Send a binlog filename in the ACK packet that is > FN_LENGTH: * 4 byte mysql-packet-header * 0xef as semi-sync magic * 8 byte log-pos * rest is log-filename like: [0000] 09 04 00 00 ef 47 03 00 00 00 00 00 00 61 61 61 .....G.......aaa [0010] 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 aaaaaaaaaaaaaaaa [0020] 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 aaaaaaaaaaaaaaaa ... [0400] 61 61 61 61 61 61 61 61 61 61 61 61 61 aaaaaaaaaaaaa leads to as stack-overrun: Thread 20 (process 57663): #0 0x00007fff85274886 in __kill () #1 0x00007fff85314e4f in __abort () #2 0x00007fff852f952c in __stack_chk_fail () #3 0x0000000100a7da49 in ReplSemiSyncMaster::readSlaveReply () #4 0x6161616161616161 in ?? () The length of the log_file_name is not checked: int ReplSemiSyncMaster::readSlaveReply(NET *net, uint32 server_id, const char *event_buf) { ... char log_file_name[FN_REFLEN]; ... strcpy(log_file_name, (const char*)packet + REPLY_BINLOG_NAME_OFFSET); Suggested fix: Check the length of the binlog filename that it is less than FN_LENGTH.