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

Commit 1619ed47 authored by Adam Lesinski's avatar Adam Lesinski Committed by Nick Kralevich
Browse files

Fix security issues when using Parcel.setDataPosition() with untrusted input

When seeking forward in the Parcel, adding the extracted size to the Parcel.dataPosition()
can result in an overflow. Guard against this.

Bug:23909429
Change-Id: If37cdebbf05a92810300363d1a6ecd8b42b6da26
parent 3089211c
Loading
Loading
Loading
Loading
+7 −5
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package android.os;
import android.annotation.Nullable;
import android.util.ArrayMap;
import android.util.Log;
import android.util.MathUtils;

import java.io.Serializable;
import java.util.ArrayList;
@@ -1345,18 +1346,19 @@ public class BaseBundle {
     */
    void readFromParcelInner(Parcel parcel) {
        int length = parcel.readInt();
        if (length < 0) {
            throw new RuntimeException("Bad length in parcel: " + length);
        }
        readFromParcelInner(parcel, length);
    }

    private void readFromParcelInner(Parcel parcel, int length) {
        if (length == 0) {
        if (length < 0) {
            throw new RuntimeException("Bad length in parcel: " + length);

        } else if (length == 0) {
            // Empty Bundle or end of data.
            mParcelledData = EMPTY_PARCEL;
            return;
        }

        int magic = parcel.readInt();
        if (magic != BUNDLE_MAGIC) {
            //noinspection ThrowableInstanceNeverThrown
@@ -1366,7 +1368,7 @@ public class BaseBundle {

        // Advance within this Parcel
        int offset = parcel.dataPosition();
        parcel.setDataPosition(offset + length);
        parcel.setDataPosition(MathUtils.addOrThrow(offset, length));

        Parcel p = Parcel.obtain();
        p.setDataPosition(0);
+8 −2
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@

package android.os;

import android.util.MathUtils;

/**
 * Parcelable containing a raw Parcel of data.
 * @hide
@@ -33,9 +35,13 @@ public class ParcelableParcel implements Parcelable {
        mParcel = Parcel.obtain();
        mClassLoader = loader;
        int size = src.readInt();
        if (size < 0) {
            throw new IllegalArgumentException("Negative size read from parcel");
        }

        int pos = src.dataPosition();
        mParcel.appendFrom(src, src.dataPosition(), size);
        src.setDataPosition(pos + size);
        src.setDataPosition(MathUtils.addOrThrow(pos, size));
        mParcel.appendFrom(src, pos, size);
    }

    public Parcel getParcel() {
+20 −0
Original line number Diff line number Diff line
@@ -185,4 +185,24 @@ public final class MathUtils {
    public static void randomSeed(long seed) {
        sRandom.setSeed(seed);
    }

    /**
     * Returns the sum of the two parameters, or throws an exception if the resulting sum would
     * cause an overflow or underflow.
     * @throws IllegalArgumentException when overflow or underflow would occur.
     */
    public static int addOrThrow(int a, int b) throws IllegalArgumentException {
        if (b == 0) {
            return a;
        }

        if (b > 0 && a <= (Integer.MAX_VALUE - b)) {
            return a + b;
        }

        if (b < 0 && a >= (Integer.MIN_VALUE - b)) {
            return a + b;
        }
        throw new IllegalArgumentException("Addition overflow: " + a + " + " + b);
    }
}
+9 −1
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package android.media;

import android.os.Parcel;
import android.util.Log;
import android.util.MathUtils;

import java.util.Calendar;
import java.util.Collections;
@@ -332,7 +333,14 @@ import java.util.TimeZone;
            }

            // Skip to the next one.
            parcel.setDataPosition(start + size);
            try {
                parcel.setDataPosition(MathUtils.addOrThrow(start, size));
            } catch (IllegalArgumentException e) {
                Log.e(TAG, "Invalid size: " + e.getMessage());
                error = true;
                break;
            }

            bytesLeft -= size;
            ++recCount;
        }