Prevent upload from sensitive path by tobiasKaminsky · Pull Request #9645 · nextcloud/android

tobiasKaminsky · 2022-01-04T13:48:08Z

Signed-off-by: tobiasKaminsky tobias@kaminsky.me

Tests written, or not not needed

codecov · 2022-01-05T15:06:12Z

Codecov Report

Merging #9645 (d0aa5d4) into master (e88a4d9) will decrease coverage by 0.40%.
The diff coverage is 0.00%.

❗ Current head d0aa5d4 differs from pull request most recent head bb78c65. Consider uploading reports for the commit bb78c65 to get more accurate results

@@             Coverage Diff             @@
##             master   #9645      +/-   ##
===========================================
- Coverage      3.11%   2.71%   -0.41%     
+ Complexity      400     327      -73     
===========================================
  Files           505     505              
  Lines         38591   38579      -12     
  Branches       5383    5384       +1     
===========================================
- Hits           1203    1048     -155     
- Misses        37311   37481     +170     
+ Partials         77      50      -27

Impacted Files	Coverage Δ
...va/com/nextcloud/client/jobs/AccountRemovalWork.kt	`0.00% <0.00%> (ø)`
.../owncloud/android/files/services/FileUploader.java	`0.00% <0.00%> (ø)`
...loud/android/operations/DownloadFileOperation.java	`0.00% <0.00%> (ø)`
...ncloud/android/operations/RenameFileOperation.java	`0.00% <0.00%> (ø)`
...ud/android/providers/DocumentsStorageProvider.java	`0.00% <0.00%> (ø)`
...m/owncloud/android/services/OperationsService.java	`0.00% <0.00%> (ø)`
...ndroid/ui/activity/RichDocumentsEditorWebView.java	`0.00% <0.00%> (ø)`
...id/ui/asynctasks/CopyAndUploadContentUrisTask.java	`0.00% <0.00%> (ø)`
...a/com/owncloud/android/utils/FileStorageUtils.java	`6.37% <ø> (+0.07%)`	⬆️
.../third_parties/daveKoeller/AlphanumComparator.java	`0.00% <0.00%> (-85.72%)`	⬇️
... and 32 more

AlvaroBrey · 2022-01-07T09:27:03Z

/rebase

AlvaroBrey · 2022-01-07T12:27:39Z

/rebase

AlvaroBrey · 2022-01-10T14:22:56Z

/rebase

AlvaroBrey

com.owncloud.android.files.services.FileUploaderIT > testKeepCancelStatic[android(AVD) - 8.1.0] �[31mFAILED �[0m
	junit.framework.AssertionFailedError
	at junit.framework.Assert.fail(Assert.java:48)

com.owncloud.android.files.services.FileUploaderIT > testKeepBothStatic[android(AVD) - 8.1.0] �[31mFAILED �[0m
	junit.framework.AssertionFailedError
	at junit.framework.Assert.fail(Assert.java:48)

com.owncloud.android.files.services.FileUploaderIT > testKeepLocalAndOverwriteRemoteStatic[android(AVD) - 8.1.0] �[31mFAILED �[0m
	junit.framework.AssertionFailedError
	at junit.framework.Assert.fail(Assert.java:48)

com.owncloud.android.files.services.FileUploaderIT > testKeepServerStatic[android(AVD) - 8.1.0] �[31mFAILED �[0m
	junit.framework.AssertionFailedError
	at junit.framework.Assert.fail(Assert.java:48)

FileUploaderIT uploads files from the internal temporal path, which includes the package name, so it makes sense that they fail.

AlvaroBrey

Lots of tests still failing as they upload from temporal path. Perhaps we should add an exception for the temporal paths?

tobiasKaminsky · 2022-01-12T07:42:50Z

Perhaps we should add an exception for the temporal paths?

Indeed we need to do this, as we use temporary path when uploading a file from sdcard:

android/src/main/java/com/owncloud/android/operations/UploadFileOperation.java

Line 549 in 883857d

    
           String temporalPath = FileStorageUtils.getInternalTemporalPath(user.getAccountName(), mContext) +

Let me change it 👍

AlvaroBrey · 2022-01-14T12:17:17Z

Tests still failing :\ This might be harder than expected

tobiasKaminsky · 2022-02-09T09:33:33Z

/rebase

AlvaroBrey

Quick smoketest: downloads are not working

2022-03-23 12:56:44.854 12951-13004/com.nextcloud.client I/DownloadFileRemoteOperation: Download of /100_MB.pdf to /data/user/0/com.nextcloud.client/files/nextcloud/tmp/MY_SERVER/100_MB.pdf: Operation finished with HTTP status code 200 (success)
2022-03-23 12:56:44.861 12951-13004/com.nextcloud.client I/DownloadFileOperation: Download of /100_MB.pdf to /storage/emulated/0/Android/media/com.nextcloud.client/nextcloud/MY_SERVER/100_MB.pdf: Error while moving file to final directory

tobiasKaminsky · 2022-05-09T07:35:27Z

/rebase

nextcloud-android-bot · 2022-05-09T12:39:04Z

Codacy

Lint

Type	master	PR
Warnings	87	87
Errors	0	0

SpotBugs

Category	Base	New
Bad practice	28	28
Correctness	44	44
Dodgy code	360	360
Experimental	1	1
Internationalization	9	9
Multithreaded correctness	9	9
Performance	66	66
Security	29	29
Total	546	546

github-actions · 2022-05-09T12:51:42Z

blue-Light-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/9645-Screenshot-blue-Light-12-51

AlvaroBrey

Nice! Needs a bit of cleanup though

app/src/androidTest/java/com/owncloud/android/files/services/FileUploaderIT.kt

app/src/androidTest/java/com/owncloud/android/utils/SyncedFolderUtilsTest.kt

app/src/main/java/com/owncloud/android/files/services/FileUploader.java

app/src/main/java/com/owncloud/android/operations/DownloadFileOperation.java

app/src/main/java/com/owncloud/android/operations/RenameFileOperation.java

app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

github-actions · 2022-07-20T09:52:28Z

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/9645.apk

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

github-actions · 2022-07-20T09:58:33Z

Codacy

Lint

Type	master	PR
Warnings	85	85
Errors	0	0

SpotBugs

Category	Base	New
Bad practice	29	29
Correctness	46	46
Dodgy code	353	353
Experimental	1	1
Internationalization	9	9
Multithreaded correctness	9	9
Performance	59	59
Security	28	28
Total	534	534

nextcloud-android-bot · 2022-07-20T10:05:16Z

master-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/5552-IT-master-10-04

nextcloud-android-bot · 2022-07-20T10:05:31Z

stable-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/5552-IT-stable-10-04

AlvaroBrey · 2022-08-08T08:34:35Z

Superseded by #10544

tobiasKaminsky added the 3. to review label Jan 4, 2022

tobiasKaminsky requested a review from AlvaroBrey January 4, 2022 13:48

AlvaroBrey approved these changes Jan 4, 2022

View reviewed changes

nextcloud-command force-pushed the sensitivePath branch from 4092f9c to 7327800 Compare January 7, 2022 09:27

nextcloud-command force-pushed the sensitivePath branch from 7327800 to 7008d73 Compare January 7, 2022 12:28

nextcloud-command force-pushed the sensitivePath branch from 7008d73 to d554592 Compare January 10, 2022 14:24

AlvaroBrey requested changes Jan 10, 2022

View reviewed changes

tobiasKaminsky requested a review from AlvaroBrey January 11, 2022 15:00

AlvaroBrey requested changes Jan 11, 2022

View reviewed changes

tobiasKaminsky requested a review from AlvaroBrey January 12, 2022 08:28

tobiasKaminsky force-pushed the sensitivePath branch 2 times, most recently from 4f28c4f to de5299d Compare January 13, 2022 07:14

tobiasKaminsky force-pushed the sensitivePath branch from de5299d to bd780c5 Compare January 17, 2022 09:21

nextcloud-command force-pushed the sensitivePath branch from d0aa5d4 to bb78c65 Compare February 9, 2022 09:34

tobiasKaminsky force-pushed the sensitivePath branch from bb78c65 to deb5af0 Compare February 9, 2022 14:38

tobiasKaminsky force-pushed the sensitivePath branch from 6dc648d to 9cbae23 Compare March 23, 2022 07:07

AlvaroBrey force-pushed the sensitivePath branch 2 times, most recently from bbfc57c to 0d3a07e Compare March 23, 2022 09:39

AlvaroBrey requested changes Mar 23, 2022

View reviewed changes

nextcloud-command force-pushed the sensitivePath branch from a279e00 to 78d14d3 Compare May 9, 2022 07:36

tobiasKaminsky force-pushed the sensitivePath branch from 78d14d3 to b792a4e Compare May 9, 2022 08:54

tobiasKaminsky requested a review from AlvaroBrey May 9, 2022 13:13

AlvaroBrey requested changes May 9, 2022

View reviewed changes

tobiasKaminsky added 2. developing and removed 3. to review labels Jul 5, 2022

tobiasKaminsky force-pushed the sensitivePath branch 2 times, most recently from 3609c59 to 77a9a37 Compare July 20, 2022 06:11

tobiasKaminsky requested a review from AlvaroBrey July 20, 2022 06:12

Prevent upload from sensitive path

c42bfcf

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

tobiasKaminsky force-pushed the sensitivePath branch from 77a9a37 to c42bfcf Compare July 20, 2022 09:38

AlvaroBrey approved these changes Jul 20, 2022

View reviewed changes

AlvaroBrey mentioned this pull request Jul 20, 2022

Prevent URI uploads from external apps that target private storage #10544

Merged

1 task

AlvaroBrey closed this Aug 8, 2022

tobiasKaminsky deleted the sensitivePath branch September 23, 2022 14:09

Uh oh!

Comments

Conversation

tobiasKaminsky commented Jan 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AlvaroBrey commented Jan 7, 2022

Uh oh!

AlvaroBrey commented Jan 7, 2022

Uh oh!

AlvaroBrey commented Jan 10, 2022

Uh oh!

AlvaroBrey left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlvaroBrey left a comment

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky commented Jan 12, 2022

Uh oh!

AlvaroBrey commented Jan 14, 2022

Uh oh!

tobiasKaminsky commented Feb 9, 2022

Uh oh!

AlvaroBrey left a comment

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky commented May 9, 2022

Uh oh!

nextcloud-android-bot commented May 9, 2022

Codacy

Lint

SpotBugs

Uh oh!

github-actions bot commented May 9, 2022

Uh oh!

AlvaroBrey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 20, 2022

Uh oh!

github-actions bot commented Jul 20, 2022

Codacy

Lint

SpotBugs

Uh oh!

nextcloud-android-bot commented Jul 20, 2022

Uh oh!

nextcloud-android-bot commented Jul 20, 2022

Uh oh!

AlvaroBrey commented Aug 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tobiasKaminsky commented Jan 4, 2022 •

edited

Loading

codecov bot commented Jan 5, 2022 •

edited

Loading

AlvaroBrey left a comment •

edited

Loading