You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
During a review, I located an off-by-one error that causes an invalid memory read (segmentation fault). Since I've determined that the address read can't be manipulated, this is only a denial-of-service, and highly unlikely to be exploited beyond this in the wild. The vector here, i.e. the way an attacker might use this bug, is by crafting a malicious configuration file and convincing users to use it - however, since the Command module exists, I don't think this bug has any impact as a security issue, thus this public PR to fix it.
All that's required to trigger this invalid memory read is passing the following config file to fastfetch:
I will likely upload a more detailed breakdown of this bug to my blog (see profile) in the coming days, but I figured I should just get a fix in sooner rather than later.
I wonder how it can be a security issue, as arr[UINT32_MAX] should likely ( if not always ) be a segfault, but it doesn't read or write valid memories.
I wonder how it can be a security issue, as arr[UINT32_MAX] should likely ( if not always ) be a segfault, but it doesn't read or write valid memories.
If index is UINT32_MAX, it triggers the index > numArgs check, since numArgs is always 8 for the terminal module, which leads to arguments[UINT32_MAX] never being accessed.
However, an attacker would need to control what index - 1 resolved to much more than is possible here to be able to exploit the arbitrary read, which is not possible due to checks in getArgumentIndex, so it's not an exploitable security issue, just a bug.
Thanks.
I wonder how it can be a security issue, as arr[UINT32_MAX] should likely ( if not always ) be a segfault, but it doesn't read or write valid memories.
If index is UINT32_MAX, it triggers the index > numArgs check, since numArgs is always 8 for the terminal module, which leads to arguments[UINT32_MAX] never being accessed.
However, an attacker would need to control what index - 1 resolved to much more than is possible here to be able to exploit the arbitrary read, which is not possible due to checks in getArgumentIndex, so it's not an exploitable security issue, just a bug.
I meant, the type of index is uint32_t, when index == 0, index - 1 is equals to UINT32_MAX.
I wonder how it can be a security issue, as arr[UINT32_MAX] should likely ( if not always ) be a segfault, but it doesn't read or write valid memories.
If index is UINT32_MAX, it triggers the index > numArgs check, since numArgs is always 8 for the terminal module, which leads to arguments[UINT32_MAX] never being accessed.
However, an attacker would need to control what index - 1 resolved to much more than is possible here to be able to exploit the arbitrary read, which is not possible due to checks in getArgumentIndex, so it's not an exploitable security issue, just a bug.
I meant, the type of index is uint32_t, when index == 0, index - 1 is equals to UINT32_MAX.
I see what's going on here. You're right that when index == 0, index - 1 = UINT32_MAX, but that bypasses the earlier check that index > numArgs (where numArgs is 8 in the context of this bug). In other words, if getArgumentIndex returns UINT32_MAX earlier, that index > numArgs check would be triggered and the dereference would not occur, but since index is 0, the check is not triggered and the dereference occurs with a value of index - 1, which is in that case UINT32_MAX. This causes the segmentation fault, which as I said earlier, I don't believe is in this case exploitable as a security issue, and is just a local denial of service bug.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ndex
Summary
During a review, I located an off-by-one error that causes an invalid memory read (segmentation fault). Since I've determined that the address read can't be manipulated, this is only a denial-of-service, and highly unlikely to be exploited beyond this in the wild. The vector here, i.e. the way an attacker might use this bug, is by crafting a malicious configuration file and convincing users to use it - however, since the
Commandmodule exists, I don't think this bug has any impact as a security issue, thus this public PR to fix it.All that's required to trigger this invalid memory read is passing the following config file to
fastfetch:{ "$schema": "https://github.com/fastfetch-cli/fastfetch/raw/dev/doc/json_schema.json", "modules": [ { "type": "terminal", "format": "{?0}│" } ] }I will likely upload a more detailed breakdown of this bug to my blog (see profile) in the coming days, but I figured I should just get a fix in sooner rather than later.
UPDATE: This detailed breakdown can now be found here: https://s0s.sh/fuzzing-fastfetch-part-1/
Related issue (required for new logos for new distros)
Fixes #2193
Changes
Checklist