ioLibrary_Driver icon indicating copy to clipboard operation
ioLibrary_Driver copied to clipboard

socket.c out-of-bound write in some architectures.

Open AvatarBecker opened this issue 2 years ago • 0 comments

A uint32_t variable is used as an array of uint8_t to try and store an IP address.

int8_t socket(uint8_t sn, uint8_t protocol, uint16_t port, uint8_t flag)
{
	CHECK_SOCKNUM();
	switch(protocol)
	{
      case Sn_MR_TCP :
         {
            //M20150601 : Fixed the warning - taddr will never be NULL
		    /*
            uint8_t taddr[4];
            getSIPR(taddr);
            */
            uint32_t taddr;         // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< THIS VARIABLE
            getSIPR((uint8_t*)&taddr);
            if(taddr == 0) return SOCKERR_SOCKINIT;
	    break;
         }
         // ...

The problem is that the bytes are not always juxtaposed in that way in an array and, if they aren't, an out-of-bound write occurs, which overwrites other variables on the stack. For example the C2000 architecture is widely used for power electronics, and its smallest addressable cell is 16 bits. I was getting a bug where my port number would be the last byte of my IP, e.g. IP 192.168.0.150 would give port 150 no matter what.

I can see it was done because the word is tested against zero, but you could read the address as a byte array and make shifts, like it's done elsewhere in the file, like in connect():

int8_t connect(uint8_t sn, uint8_t * addr, uint16_t port)
{
    CHECK_SOCKNUM();
    CHECK_SOCKMODE(Sn_MR_TCP);
    CHECK_SOCKINIT();
    //M20140501 : For avoiding fatal error on memory align mismatched
    //if( *((uint32_t*)addr) == 0xFFFFFFFF || *((uint32_t*)addr) == 0) return SOCKERR_IPINVALID;
   {
        uint32_t taddr;
        taddr = ((uint32_t)addr[0] & 0x000000FF);
        taddr = (taddr << 8) + ((uint32_t)addr[1] & 0x000000FF);
        taddr = (taddr << 8) + ((uint32_t)addr[2] & 0x000000FF);
        taddr = (taddr << 8) + ((uint32_t)addr[3] & 0x000000FF);
        if( taddr == 0xFFFFFFFF || taddr == 0) return SOCKERR_IPINVALID;
   }
   // ...

AvatarBecker avatar Nov 30 '23 16:11 AvatarBecker