EnAccess / OpenPAYGO-HW

Open Source token system with hardware and software agnostic technology for enabling PAYGO functionality in any device & making products PAYGO compatible.
https://enaccess.org/materials/openpaygotoken/
Apache License 2.0
7 stars 15 forks source link

MarkCountAsUsed() function is not working properly #10

Open JoLo33 opened 2 years ago

JoLo33 commented 2 years ago

Hey,

it is possible to enter an older Add token than an entered Set token in the C implementation. I figured out that the MarkCountAsUsed() function is not working properly.

The macro CHECK_BIT(variable, position) is not good.

 #define CHECK_BIT(variable, position) (((variable) >> (position)) & 1)
   int test = 0;    
  test = CHECK_BIT(0b111, 2);

test will be 4. That's ok if you use the check only in if() but it will be used also in the SET_BIT(variable,position,value) as value and the value should only be 1 or 0. That leads to

#define SET_BIT(variable,position,value) (variable & ~(1<<position)) | (value<<position)
   #define CHECK_BIT(variable, position) (((variable) >> (position)) & 1)
   int test = 0;    
   test = SET_BIT(0b0,0,CHECK_BIT(0b111, 2));

and test will be 0b100 and not 0b1 as expected.

Also the for loop is not working correct. The function will still not work correctly with the new CHECK_BIT macro from above.

There are two problems:

  1. uint16_t maxcount = 0;
    uint16_t UsedCounts = 0;
    
    MarkCountAsUsed(4,  &maxcount, &UsedCounts, 1);
    MarkCountAsUsed(8,  &maxcount, &UsedCounts, 1);
    MarkCountAsUsed(16, &maxcount, &UsedCounts, 1);

    After this, UsedCounts is 0b10000000, but that is wrong because counts 4 and 8 are now marked as unused again.

  2. MarkCountAsUsed(33, &maxcount, &UsedCounts, 1);
    MarkCountAsUsed(34, &maxcount, &UsedCounts, 1);

    After the first example, the counts 33 and 34 will should be marked. Count 33 will set UsedCounts to 0b1111111111111111, that's correct, but count 34 will set it to 0b111111111111101 so that count 32 is marked as unused, which is not correct.

If the for loop will be changed to following, it will work

for(int i=RelativeCount+1; i < MAX_UNUSED_OLDER_TOKENS - RelativeCount; i++) {
                NewUsedCount = SET_BIT(NewUsedCount, i, CHECK_BIT(*UsedCounts, i-RelativeCount));
            }

Regards Jonas

benmod commented 1 year ago

Hi! Thanks for reporting the issue, can you provide a bit of details on the type of MCU and compiler you are using? This might be an endianness issue.

JoLo33 commented 1 year ago

Hey, of course:

MCU: STM32F401RETx IDE: CubeIDE V: 1.5.1
Toolchain Version: Type: GBU Tools for STM32 / Version: 7-2018-q2-update STM32CubeMX: 6.1.1-RC2
Firmware Package: STM32Cube FW_F4 V1.25.2
FreeRTOS: 10.0.1
CMSIS-RTOS: 1.02
Optimization level: -Og