Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0cce5355 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Sanitize display names, keep extensions intact. 

When creating or renaming files on external storage, sanitize the
requested display names to be valid FAT filenames.  This also fixes
a handful of directory traversal bugs.

Also relax logic around generating display names to allow any
extension which maps to the requested MIME type.  Tests to verify.

Bug: 18512473, 18504132
Change-Id: I89e632019ee145f53d9d9d2050932f8939a756af
parent 48e17629

core/java/android/os/FileUtils.java

+74 −6
Original line number Diff line number Diff line
@@ -17,9 +17,8 @@ 
package android.os;



import android.system.ErrnoException;

import android.text.TextUtils;

import android.system.Os;

import android.system.OsConstants;

import android.text.TextUtils;

import android.util.Log;

import android.util.Slog;
@@ -403,21 +402,90 @@ public class FileUtils { 
        return success;

    }



    private static boolean isValidExtFilenameChar(char c) {

        switch (c) {

            case '\0':

            case '/':

                return false;

            default:

                return true;

        }

    }



    /**

     * Assert that given filename is valid on ext4.

     * Check if given filename is valid for an ext4 filesystem.

     */

    public static boolean isValidExtFilename(String name) {

        return (name != null) && name.equals(buildValidExtFilename(name));

    }



    /**

     * Mutate the given filename to make it valid for an ext4 filesystem,

     * replacing any invalid characters with "_".

     */

    public static String buildValidExtFilename(String name) {

        if (TextUtils.isEmpty(name) || ".".equals(name) || "..".equals(name)) {

            return false;

            return "(invalid)";

        }

        final StringBuilder res = new StringBuilder(name.length());

        for (int i = 0; i < name.length(); i++) {

            final char c = name.charAt(i);

            if (c == '\0' || c == '/') {

                return false;

            if (isValidExtFilenameChar(c)) {

                res.append(c);

            } else {

                res.append('_');

            }

        }

        return res.toString();

    }



    private static boolean isValidFatFilenameChar(char c) {

        if ((0x00 <= c && c <= 0x1f)) {

            return false;

        }

        switch (c) {

            case '"':

            case '*':

            case '/':

            case ':':

            case '<':

            case '>':

            case '?':

            case '\\':

            case '|':

            case 0x7F:

                return false;

            default:

                return true;

        }

    }



    /**

     * Check if given filename is valid for a FAT filesystem.

     */

    public static boolean isValidFatFilename(String name) {

        return (name != null) && name.equals(buildValidFatFilename(name));

    }



    /**

     * Mutate the given filename to make it valid for a FAT filesystem,

     * replacing any invalid characters with "_".

     */

    public static String buildValidFatFilename(String name) {

        if (TextUtils.isEmpty(name) || ".".equals(name) || "..".equals(name)) {

            return "(invalid)";

        }

        final StringBuilder res = new StringBuilder(name.length());

        for (int i = 0; i < name.length(); i++) {

            final char c = name.charAt(i);

            if (isValidFatFilenameChar(c)) {

                res.append(c);

            } else {

                res.append('_');

            }

        }

        return res.toString();

    }



    public static String rewriteAfterRename(File beforeDir, File afterDir, String path) {

        if (path == null) return null;

core/tests/coretests/src/android/os/FileUtilsTest.java

+45 −0
Original line number Diff line number Diff line
@@ -180,6 +180,51 @@ public class FileUtilsTest extends AndroidTestCase { 
        assertDirContents("file1", "file2");

    }



    public void testValidExtFilename() throws Exception {

        assertTrue(FileUtils.isValidExtFilename("a"));

        assertTrue(FileUtils.isValidExtFilename("foo.bar"));

        assertTrue(FileUtils.isValidExtFilename("foo bar.baz"));

        assertTrue(FileUtils.isValidExtFilename("foo.bar.baz"));

        assertTrue(FileUtils.isValidExtFilename(".bar"));

        assertTrue(FileUtils.isValidExtFilename("foo~!@#$%^&*()_[]{}+bar"));



        assertFalse(FileUtils.isValidExtFilename(null));

        assertFalse(FileUtils.isValidExtFilename("."));

        assertFalse(FileUtils.isValidExtFilename("../foo"));

        assertFalse(FileUtils.isValidExtFilename("/foo"));



        assertEquals(".._foo", FileUtils.buildValidExtFilename("../foo"));

        assertEquals("_foo", FileUtils.buildValidExtFilename("/foo"));

        assertEquals("foo_bar", FileUtils.buildValidExtFilename("foo\0bar"));

        assertEquals(".foo", FileUtils.buildValidExtFilename(".foo"));

        assertEquals("foo.bar", FileUtils.buildValidExtFilename("foo.bar"));

    }



    public void testValidFatFilename() throws Exception {

        assertTrue(FileUtils.isValidFatFilename("a"));

        assertTrue(FileUtils.isValidFatFilename("foo bar.baz"));

        assertTrue(FileUtils.isValidFatFilename("foo.bar.baz"));

        assertTrue(FileUtils.isValidFatFilename(".bar"));

        assertTrue(FileUtils.isValidFatFilename("foo.bar"));

        assertTrue(FileUtils.isValidFatFilename("foo bar"));

        assertTrue(FileUtils.isValidFatFilename("foo+bar"));

        assertTrue(FileUtils.isValidFatFilename("foo,bar"));



        assertFalse(FileUtils.isValidFatFilename("foo*bar"));

        assertFalse(FileUtils.isValidFatFilename("foo?bar"));

        assertFalse(FileUtils.isValidFatFilename("foo<bar"));

        assertFalse(FileUtils.isValidFatFilename(null));

        assertFalse(FileUtils.isValidFatFilename("."));

        assertFalse(FileUtils.isValidFatFilename("../foo"));

        assertFalse(FileUtils.isValidFatFilename("/foo"));



        assertEquals(".._foo", FileUtils.buildValidFatFilename("../foo"));

        assertEquals("_foo", FileUtils.buildValidFatFilename("/foo"));

        assertEquals(".foo", FileUtils.buildValidFatFilename(".foo"));

        assertEquals("foo.bar", FileUtils.buildValidFatFilename("foo.bar"));

        assertEquals("foo_bar__baz", FileUtils.buildValidFatFilename("foo?bar**baz"));

    }



    private void touch(String name, long age) throws Exception {

        final File file = new File(mDir, name);

        file.createNewFile();

packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java

+72 −39
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ import android.util.Log; 
import android.webkit.MimeTypeMap;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;

import com.google.android.collect.Lists;

import com.google.android.collect.Maps;
@@ -53,6 +54,7 @@ import java.util.ArrayList; 
import java.util.HashMap;

import java.util.LinkedList;

import java.util.Map;

import java.util.Objects;



public class ExternalStorageProvider extends DocumentsProvider {

    private static final String TAG = "ExternalStorage";
@@ -313,27 +315,19 @@ public class ExternalStorageProvider extends DocumentsProvider { 
    @Override

    public String createDocument(String docId, String mimeType, String displayName)

            throws FileNotFoundException {

        displayName = FileUtils.buildValidFatFilename(displayName);



        final File parent = getFileForDocId(docId);

        if (!parent.isDirectory()) {

            throw new IllegalArgumentException("Parent document isn't a directory");

        }



        File file;

        final File file = buildUniqueFile(parent, mimeType, displayName);

        if (Document.MIME_TYPE_DIR.equals(mimeType)) {

            file = new File(parent, displayName);

            if (!file.mkdir()) {

                throw new IllegalStateException("Failed to mkdir " + file);

            }

        } else {

            displayName = removeExtension(mimeType, displayName);

            file = new File(parent, addExtension(mimeType, displayName));



            // If conflicting file, try adding counter suffix

            int n = 0;

            while (file.exists() && n++ < 32) {

                file = new File(parent, addExtension(mimeType, displayName + " (" + n + ")"));

            }



            try {

                if (!file.createNewFile()) {

                    throw new IllegalStateException("Failed to touch " + file);
@@ -342,11 +336,78 @@ public class ExternalStorageProvider extends DocumentsProvider { 
                throw new IllegalStateException("Failed to touch " + file + ": " + e);

            }

        }



        return getDocIdForFile(file);

    }



    private static File buildFile(File parent, String name, String ext) {

        if (TextUtils.isEmpty(ext)) {

            return new File(parent, name);

        } else {

            return new File(parent, name + "." + ext);

        }

    }



    @VisibleForTesting

    public static File buildUniqueFile(File parent, String mimeType, String displayName)

            throws FileNotFoundException {

        String name;

        String ext;



        if (Document.MIME_TYPE_DIR.equals(mimeType)) {

            name = displayName;

            ext = null;

        } else {

            String mimeTypeFromExt;



            // Extract requested extension from display name

            final int lastDot = displayName.lastIndexOf('.');

            if (lastDot >= 0) {

                name = displayName.substring(0, lastDot);

                ext = displayName.substring(lastDot + 1);

                mimeTypeFromExt = MimeTypeMap.getSingleton().getMimeTypeFromExtension(

                        ext.toLowerCase());

            } else {

                name = displayName;

                ext = null;

                mimeTypeFromExt = null;

            }



            if (mimeTypeFromExt == null) {

                mimeTypeFromExt = "application/octet-stream";

            }



            final String extFromMimeType = MimeTypeMap.getSingleton().getExtensionFromMimeType(

                    mimeType);

            if (Objects.equals(mimeType, mimeTypeFromExt) || Objects.equals(ext, extFromMimeType)) {

                // Extension maps back to requested MIME type; allow it

            } else {

                // No match; insist that create file matches requested MIME

                name = displayName;

                ext = extFromMimeType;

            }

        }



        File file = buildFile(parent, name, ext);



        // If conflicting file, try adding counter suffix

        int n = 0;

        while (file.exists()) {

            if (n++ >= 32) {

                throw new FileNotFoundException("Failed to create unique file");

            }

            file = buildFile(parent, name + " (" + n + ")", ext);

        }



        return file;

    }



    @Override

    public String renameDocument(String docId, String displayName) throws FileNotFoundException {

        // Since this provider treats renames as generating a completely new

        // docId, we're okay with letting the MIME type change.

        displayName = FileUtils.buildValidFatFilename(displayName);



        final File before = getFileForDocId(docId);

        final File after = new File(before.getParentFile(), displayName);

        if (after.exists()) {
@@ -482,34 +543,6 @@ public class ExternalStorageProvider extends DocumentsProvider { 
        return "application/octet-stream";

    }



    /**

     * Remove file extension from name, but only if exact MIME type mapping

     * exists. This means we can reapply the extension later.

     */

    private static String removeExtension(String mimeType, String name) {

        final int lastDot = name.lastIndexOf('.');

        if (lastDot >= 0) {

            final String extension = name.substring(lastDot + 1).toLowerCase();

            final String nameMime = MimeTypeMap.getSingleton().getMimeTypeFromExtension(extension);

            if (mimeType.equals(nameMime)) {

                return name.substring(0, lastDot);

            }

        }

        return name;

    }



    /**

     * Add file extension to name, but only if exact MIME type mapping exists.

     */

    private static String addExtension(String mimeType, String name) {

        final String extension = MimeTypeMap.getSingleton()

                .getExtensionFromMimeType(mimeType);

        if (extension != null) {

            return name + "." + extension;

        }

        return name;

    }



    private void startObserving(File file, Uri notifyUri) {

        synchronized (mObservers) {

            DirectoryObserver observer = mObservers.get(file);

packages/ExternalStorageProvider/tests/Android.mk

0 → 100644
+16 −0
Original line number Diff line number Diff line 


LOCAL_PATH := $(call my-dir)

include $(CLEAR_VARS)



LOCAL_MODULE_TAGS := tests



LOCAL_SRC_FILES := $(call all-java-files-under, src)



LOCAL_JAVA_LIBRARIES := android.test.runner



LOCAL_PACKAGE_NAME := ExternalStorageProviderTests

LOCAL_INSTRUMENTATION_FOR := ExternalStorageProvider



LOCAL_CERTIFICATE := platform



include $(BUILD_PACKAGE)

packages/ExternalStorageProvider/tests/AndroidManifest.xml

0 → 100644
+13 −0
Original line number Diff line number Diff line 
<?xml version="1.0" encoding="utf-8"?>

<manifest xmlns:android="http://schemas.android.com/apk/res/android"

    package="com.android.externalstorage.tests">



    <application>

        <uses-library android:name="android.test.runner" />

    </application>



    <instrumentation android:name="android.test.InstrumentationTestRunner"

        android:targetPackage="com.android.externalstorage"

        android:label="Tests for ExternalStorageProvider" />



</manifest>