Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crashloop when attachment with size as string is found #7406

Closed
andir opened this issue Mar 4, 2023 · 0 comments · Fixed by #7429
Closed

Crashloop when attachment with size as string is found #7406

andir opened this issue Mar 4, 2023 · 0 comments · Fixed by #7429
Assignees
Labels
A-Media crash O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Critical Prevents work, causes data loss and/or has no workaround T-Defect Something isn't working: bugs, crashes, hangs and other reported problems

Comments

@andir
Copy link

andir commented Mar 4, 2023

Steps to reproduce

  1. Join a matrix channel
  2. Have someone post an attachment into the channel that encodes the file lenght in the FileInfo part (https://spec.matrix.org/v1.6/client-server-api/#mfile) of the m.file event as string rather than integer.
  3. Open element on your iPhone / Login to a new profile
  4. The app will now crash whenever you open it without any chance to submit a crash report.

Outcome

What did you expect?

I would not expect the app to crash. Not rendering a message, rendering it with an error or just the missing size would be deisreable. Continously crashing the app is not.

What happened instead?

The crash happens in the code responsible for formatting the event to text. There the code assumes that the object in the dictonary is always a number and it is safe to convert it into a long value. The code is missing required type checks in at least these cases.

I've validated the situation using the below patch which fixed it in my simulator. This was my very first piece of Objective-C/iOS development so I don't think this is good enough to go in as a PR.

From 7f8a37836dc601864897629071c917b879d8cd13 Mon Sep 17 00:00:00 2001
From: Andreas Rammhold <andreas@rammhold.de>
Date: Sat, 4 Mar 2023 11:58:31 +0100
Subject: [PATCH] Fix crash when receiving messages that contain file sizes as
 string
---
 .../Utils/EventFormatter/MXKEventFormatter.m          | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Riot/Modules/MatrixKit/Utils/EventFormatter/MXKEventFormatter.m b/Riot/Modules/MatrixKit/Utils/EventFormatter/MXKEventFormatter.m
index 9f34bcbb6..e13bc71ba 100644
--- a/Riot/Modules/MatrixKit/Utils/EventFormatter/MXKEventFormatter.m
+++ b/Riot/Modules/MatrixKit/Utils/EventFormatter/MXKEventFormatter.m
@@ -1365,10 +1365,15 @@ static NSString *const kRepliedTextPattern = @"<mx-reply>.*<blockquote>.*<br>(.*
                             NSDictionary *fileInfo = contentToUse[@"info"];
                             if (fileInfo)
                             {
-                                NSNumber *fileSize = fileInfo[@"size"];
-                                if (fileSize)
+                                NSObject* fileSize = fileInfo[@"size"];
+                                if ([fileSize isKindOfClass:[NSNumber class]])
                                 {
-                                    body = [NSString stringWithFormat:@"%@ (%@)", body, [MXTools fileSizeToString: fileSize.longValue]];
+                                    NSNumber* nFileSize = (NSNumber*) fileSize;
+                                    body = [NSString stringWithFormat:@"%@ (%@)", body, [MXTools fileSizeToString: nFileSize.longValue]];
+                                } else if ([fileSize isKindOfClass:[NSString class]]) {
+                                    body = [NSString stringWithFormat:@"%@ (%@)", body, fileSize];
+                                } else {
+                                    // FIXME: add some logging that we can't make sense of the file size attribute?
                                 }
                             }
                         }
-- 
2.39.1

Your phone model

IPhone 14 & IPhone 14 Pro in Simulator

Operating system version

iOS16.3

Application version

latest develop branch (47d50d7)

Homeserver

Synapse 1.76 / my personal instance

Will you send logs?

I did once after turning the homeserver off as otherwise the app continously crashes.

@andir andir added the T-Defect Something isn't working: bugs, crashes, hangs and other reported problems label Mar 4, 2023
@RiotRobot RiotRobot added this to Incoming in Issue triage Mar 4, 2023
@pixlwave pixlwave added crash A-Media O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround S-Critical Prevents work, causes data loss and/or has no workaround and removed S-Major Severely degrades major functionality or product features, with no satisfactory workaround labels Mar 10, 2023
@RiotRobot RiotRobot moved this from Incoming to Triaged in Issue triage Mar 10, 2023
@manuroe manuroe self-assigned this Mar 14, 2023
manuroe added a commit that referenced this issue Mar 15, 2023
#7406

By using value type checker methods.

With this fix, attachments with a wrong size format are still displayed in the timeline. Only the size is omitted
Issue triage automation moved this from Triaged to Closed Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Media crash O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Critical Prevents work, causes data loss and/or has no workaround T-Defect Something isn't working: bugs, crashes, hangs and other reported problems
Projects
Development

Successfully merging a pull request may close this issue.

3 participants