paullouisageneau / libjuice

JUICE is a UDP Interactive Connectivity Establishment library
Mozilla Public License 2.0
403 stars 75 forks source link

Misalignment issue during STUN data reading #232

Closed Abeja27 closed 7 months ago

Abeja27 commented 7 months ago

Hello,

I'm currently developing an application using libdatachannel and I ran into an issue when receiving a WebRTC connection. The sanitizer flagged a problem related to adressess alignment, and the specific error is as follows:

libdatachannel/deps/libjuice/src/stun.c:920:25: runtime error: load of misaligned address 0x7f23096936e4 for type 
'uint64_t', which requires 8 byte alignment 0x7f23096936e4: note: pointer points here
           80 29 00 08 7b 65 03 f1  3c 30 a1 1a 00 08 00 14  06 2c 16 76 77 83 55 63  e3 48 c6 f1 18 d3 63 ee
                       ^

Though my knowledge in these areas is limited, I checked the source code and think I've identified the problem. It seems that the stun_attr struct is 8 bytes aligned, but the value member is only 4 bytes due to the additional 4 bytes reserved for the type and length members. To address this, I experimented with modifying the code using uint32_t * pointers, and it appears to resolve the issue for me:

// Current code
// msg->ice_controlled = ntohll(*((uint64_t *)attr->value));

// My test
   uint32_t* value32 = (uint32_t *)attr->value;
   msg->ice_controlled = ntohll(((uint64_t)value32[1] << 32) | value32[0]);

I'd appreciate confirmation if my understanding is correct or if I've overlooked something. Just so you know, I'm using a 64-bit machine.

Thanks in advance for your time.

paullouisageneau commented 7 months ago

Good catch, the STUN parser indeed reads a 64-bit value aligned on 32 bits only, which is undefined behavior. Your fix looks good to me, would you like to open a PR addressing both ice_controlling and ice_controlled?

Abeja27 commented 7 months ago

Okey, I'll do it tomorrow

Abeja27 commented 7 months ago

Done! I've also changed the reservation_token. Let me know if you need anything else or something is wrong.