[SECURITY] out-of-bound in commandoption.c

[RootUp]

Summary

While fuzzing fastfetch (4175dfd) it was found that the application suffers from out-of-bound due to lack of input validation, allowing application to crash via a crafted configuration files leading to denial or service or code execution.

ASAN

==3290000==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x007900000078 (pc 0x00000042cd55 bp 0x7fffffffe330 sp 0x7fffffffe090 T3290000)
==3290000==The signal is caused by a READ memory access.
    #0 0x42cd55 in ffParseModuleOptions /fastfetch/src/common/commandoption.c:16:77
    #1 0x428d39 in parseOption /fastfetch/src/fastfetch.c:737:9
    #2 0x42b86c in parseConfigFile /fastfetch/src/fastfetch.c:397:13
    #3 0x427bc6 in optionParseConfigFile /fastfetch/src/fastfetch.c:514:47
    #4 0x427bc6 in parseCommand /fastfetch/src/fastfetch.c:662:9
    #5 0x4278d3 in parseArguments /fastfetch/src/fastfetch.c:795:13
    #6 0x427014 in main /fastfetch/src/fastfetch.c:874:5
    #7 0x7ffff7c4e082 in __libc_start_main /build/glibc-e2p3jK/glibc-2.31/csu/../csu/libc-start.c:308:16
    #8 0x406bed in _start (/fastfetch/build/fastfetch+0x406bed)

UndefinedBehaviorSanitizer can not provide additional info.
SUMMARY: UndefinedBehaviorSanitizer: SEGV /fastfetch/src/common/commandoption.c:16:77 in ffParseModuleOptions
==3290000==ABORTING

Code Snippet

https://github.com/fastfetch-cli/fastfetch/blob/dev/src/common/commandoption.c#L16

bool ffParseModuleOptions(const char* key, const char* value)
{
    if (!ffStrStartsWith(key, "--") || !isalpha(key[2])) return false;

    for (FFModuleBaseInfo** modules = ffModuleInfos[toupper(key[2]) - 'A']; *modules; ++modules)
    {
        FFModuleBaseInfo* baseInfo = *modules;
        if (baseInfo->parseCommandOptions(baseInfo, key, value)) return true;
    }
    return false;
}

This issue was caused due to the toupper(key[2]) - 'A' expression, which lead to out-of-bounds in the ffModuleInfos array if key is shorter than 3 characters or key[2] is not a valid alphabetic character.

Proof-of-concept: oob.zip

[apocelipes]

Could you assign this to me? Thanks.

[CarterLi]

Taked a quick look.

This issue was caused due to the toupper(key[2]) - 'A' expression, which lead to out-of-bounds in the ffModuleInfos array

if key is shorter than 3 characters

It should not be possible because if ffStrStartsWith(key, "--") check passes key must have 2 chars at least; and if isalpha(key[2]) check passes the 3rd char of key must also exist. So that key must have at least 3 chars

or key[2] is not a valid alphabetic character.

We have isalpha(key[2]) check

[CarterLi]

I am going to bed. @apocelipes you may check it in detail if you like

[apocelipes]

I decoded the data, and found a 'Ó' (\xd3, 211). 211 - 'A' equals 146, and ffModuleInfos length is only 26. fastfetch use setlocale(LC_ALL, ""); which will use the user's locale settings. And in some locales, isalpha('Ó') will return true.

Checking the alpha to see if it is an English alphabet can fix this issue.

Here's the evidence:

Also, GCC docs said "Each kind of machine has a default for what char should be. It is either like unsigned char by default or like signed char by default", so a char in some machines can hold >= 128 values. In fact, Android-arm use the unsigned char by default.

@CarterLi

[apocelipes]

I'll send a PR to fix this issue.
However, I'm not sure is there any other place has the same type bug. (All the usages of ffModuleInfos have been fixed)

[CarterLi]

So that ctype.h is locale dependent. That's bad. I'm considering replacing the every usage of ctype functions