Copy Paste Code Strikes Again

Mar 1, 2018

A few days ago I was messing around with smbclient and I hit a null-deref when switching trees. I quickly cloned samba and began investigating. The minimal repro looked something like the following.

$ ./bin/smbclient --version
Version 4.9.0pre1-DEVELOPERBUILD
$ ./bin/smbclient -m smb2 -U <user> \\\\<ip>\\<share>
smb: \> tid
current tid is 5
smb: \> tid 1
ASAN:SIGSEGV

Essentially any time I switched trees when using SMB2 I hit a null deref. Thanks to address sanitized builds, the cause was fairly easy to track down.

if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
    ret = smb2cli_tcon_current_id(cli->smb1.tcon);
    smb2cli_tcon_set_id(cli->smb1.tcon, tid);
} else {
    ret = smb1cli_tcon_current_id(cli->smb1.tcon);
    smb1cli_tcon_set_id(cli->smb1.tcon, tid);
}

Note the use of the smb1 tree connection in the branch that clearly should be using the smb2 tree connection. I like to think this was due to a copy-paste followed by forgetting to update the tcon used in the SMB2 branch. At this point I was feeling pretty good about myself… A little too good.

A few days later I finished up some work on updating the PCI device driver for RedoxOS. I was redesigning the parsing of the PCI Configuration Space so that it could be more easily tested and could parse multiple types of headers. My PR was almost finished and I just had to allow configured drivers to request a given BAR. This part is a bit tedious and required writing two similar lines 4 times in a row. Instead of thinking up a clever vim macro or writing out manually, I just copied the two lines and pasted it a few times (2yyj3p to be exact for my fellow vim users out there). All was well and good, only I forgot to go back and change the index leaving me with this gem.

"$BAR2" if header.header_type() == PciHeaderType::GENERAL =>
    format!("{}", header.get_bar(2)),
"$BAR3" if header.header_type() == PciHeaderType::GENERAL =>
    format!("{}", header.get_bar(2)),
"$BAR4" if header.header_type() == PciHeaderType::GENERAL =>
    format!("{}", header.get_bar(2)),
"$BAR5" if header.header_type() == PciHeaderType::GENERAL =>
    format!("{}", header.get_bar(2)),

Note that we get the second BAR in each branch. Luckily jackpot51 caught my mistake and immediately posted a fix for it.