Skip to content

Fix log CRLF injection#7883

Merged
Alkarex merged 2 commits intoFreshRSS:edgefrom
Inverle:fix-log-crlf-injection
Aug 31, 2025
Merged

Fix log CRLF injection#7883
Alkarex merged 2 commits intoFreshRSS:edgefrom
Inverle:fix-log-crlf-injection

Conversation

@Inverle
Copy link
Member

@Inverle Inverle commented Aug 30, 2025

Can inject new lines into Minz_Request::paramString('challenge') f.e. by logging in with challenge x%0D%0A[Sat, 30 Aug 2025 21:39:50 +0000] [warning] --- something

Minz_Log::warning("Password mismatch for user={$username}, nonce={$nonce}, c={$challenge}");

image

If it were Minz_Request::paramString('challenge', plaintext: true) or somewhere else, HTML could be injected into level

<tr class="log-item log-<?= $log->level() ?>">

which is why its values are restricted now

@Alkarex Alkarex added this to the 1.28.0 milestone Aug 31, 2025
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
@Inverle
Copy link
Member Author

Inverle commented Aug 31, 2025

Before merging it would be useful to check if there are any other characters that may be used as line delimiter other than \r or \n (in ^)

if (preg_match('/^\[([^\[]+)\] \[([^\[]+)\] --- (.*)$/', $line, $matches)) {

edit: looks like not

@Alkarex Alkarex merged commit c44bb02 into FreshRSS:edge Aug 31, 2025
1 check passed
@Inverle Inverle deleted the fix-log-crlf-injection branch August 31, 2025 18:06
@Alkarex Alkarex modified the milestones: 1.28.0, 1.27.1 Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants