net: signed-integer-overflow in LocalServiceInfo #24049

LocalServiceInfo is defined as: https://github.com/bitcoin/bitcoin/blob/16781e1bc9f8ffc721ebea73434e0066957bc959/src/net.h#L228-L231 nScore is of type int which is signed and is 4 bytes wide on my machine (clang-12). nScore is incremented by SeenLocal during the version handshake here: https://github.com/bitcoin/bitcoin/blob/16781e1bc9f8ffc721ebea73434e0066957bc959/src/net_processing.cpp#L2603-L2606 So I believe this UB is peer-triggerable (I don't think the compiler can detect this), but I don't think it's a big deal unless there's a node out there compiled with UBSAN or ftrapv. It seems like nScore can just be changed to int64_t.

#24090 was closed with no comment. @Crypt-iQ do you want to follow up here?

I won't have time to work on this unfortunately

Ok. Labelling this "Up for grabs" for now.

I have a potential fix… It's ugly, though. And it looks like it should be UB, but I can't find a reason why.
The function which may invoke UB is (currently) defined like so: https://github.com/bitcoin/bitcoin/blob/5b6f0f31fa6ce85db3fb7f9823b1bbb06161ae32/src/net.cpp#L344-L352

++reinterpret_cast <unsigned&> (it->second.nScore);

It's ugly, I know… But it leaves everything else in this repo untouched.

Replying to #26399 (comment):

The burden to […] motivate a change […] is on the author.

The patch makes the behavior of the code defined. (See: [intro.compliance.general], [intro.abstract]) It should do so, without any conversions.

I spent over a hundred hours trying to prove it couldn't be done. First, I asked more senior engineers, chatted llvm devs to know it could [be an asan bug], then finally posted on stackoverflow.

You […] ignored previous discussions, ignored two review comments

I did? I'll have to look into that. I agree with the rest of your comment, though.

I have very little free time, and this isn't how I want to spend it. Please don't mistake my frustration for arrogance.

Since I am new to open source, I would like to know if I need to change int to int64_t for nScore or if I have to work on something else.