Alfranio Correia wrote:
> Hi Jasonh,
>
> I did not understand the patch.
> See some questions in-line.
>
The purpose of this patch is to make the semisync code safer from
possible buffer overflow attacks.
> Let's talk in the IRC.
>
> Cheers.
>
> He Zhenxing wrote:
> > #At file:///media/sdb2/hezx/work/mysql/bzr/b52748/trunk-bugfixing/ based on
> revid:jon.hauglid@stripped
> >
> > 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.
> >
> > modified:
> > plugin/semisync/semisync_master.cc
> > === modified file 'plugin/semisync/semisync_master.cc'
> > --- a/plugin/semisync/semisync_master.cc 2010-03-11 02:22:18 +0000
> > +++ b/plugin/semisync/semisync_master.cc 2010-05-24 02:57:37 +0000
> > @@ -1048,6 +1048,7 @@ int ReplSemiSyncMaster::readSlaveReply(N
> > const unsigned char *packet;
> > char log_file_name[FN_REFLEN];
> > my_off_t log_file_pos;
> > + ulong log_file_len = 0;
> > ulong packet_len;
> > int result = -1;
> >
> > @@ -1123,7 +1124,13 @@ int ReplSemiSyncMaster::readSlaveReply(N
> > }
> >
> > log_file_pos = uint8korr(packet + REPLY_BINLOG_POS_OFFSET);
> > - strcpy(log_file_name, (const char*)packet + REPLY_BINLOG_NAME_OFFSET);
> > + log_file_len = packet_len - REPLY_BINLOG_NAME_OFFSET;
>
> Is this the reserved space in the packet for the log file name?
>
REPLY_BINLOG_NAME_OFFSET is the start position for the log file name in
the reply packet, the log file name will span to the end of the packet,
so the length of the log file will be:
packet_len - REPLY_BINLOG_NAME_OFFSET
> > + if (log_file_len > FN_REFLEN)
> > + {
> > + sql_print_error("Read semi-sync reply binlog file length too large");
> > + goto l_end;
> > + }
>
> I think the "if" should be:
>
> if (FN_REFLEN > log_file_len)
>
No, here the code checks the sanity of the reply packet, if the length
of log file in the reply packet is longer than FN_REFLEN, then we
considered the reply packet is forged or corrupted, and will be
rejected.
> > + strncpy(log_file_name, (const char*)packet + REPLY_BINLOG_NAME_OFFSET,
> log_file_len);
> >
> > if (trc_level & kTraceDetail)
> > sql_print_information("%s: Got reply (%s, %lu)",
> >
>
