SQL injection through ContentProvider projection

The SQL injection through query parameters is the common security issue of any system using SQL database. Android is no different than any other system, so if you're using SQLite database in your Android app, you should always sanitize the database inputs.

Obligatory XKCD

If you are also using an exported ContentProvider, you need to take care of one more vector of attack: the projection parameter of the queries. Just like SQLiteDatabase, the ContentProvider allows the users to specify which columns they want to retrieve. It makes sense, because it reduces the amount of data fetched, which might improve performance and reduce the RAM footprint of your app. Unlike the SQLiteDatabase, the ContentProvider might be exported, which means that the external applications can query the data from it requesting an arbitrary projection, which are then turned into raw SQL queries. For example:

'Bobby Tables was here'; DROP TABLE Students; --
* FROM sqlite_master; --
* FROM non_public_table_I_found_out_about_using_previous_query; --

Basically it means that if you exposed a single uri without sanitizing the projection, you have exposed your entire db.

So how do you sanitize your projections? I've given it some thought and it seems that the only sensible thing to do is allowing only subsets of predefined set of columns.

You cannot allow any expression, because you'd allow any expressions, including SELECTs from other tables and allowing certain expressions is not a trivial task.

You shouldn't ignore the provided projection and return all columns, because one of the benefits of using projections is limiting the amount of data retrieved from database. Besides, certain widely used Google application ignores the existence of Cursor.getColumnIndex method and assumes that the columns will be returned in the same order they were specified in projection. The other app won't work correctly, and the users will probably blame you.

Android drawables stroke inconsistency

I've run into a funny little problem when creating custom drawables recently - some of the lines were crisp and some were blurred:

After few debug iterations I was able to narrow down the difference to the shapes drawn using the Canvas.drawRoundRect and Canvas.drawPath. The former looked much crispier. I've dug down to Skia classes and it turns out that they reach the same drawing function through slightly different code paths and I guess at some point some rounding is applied at one of them, but I haven't verified this.

The minimal example which demonstrates the issue are two solid XML shape drawables (which are parsed into GradientDrawables), one with radius defined in radius attribute, the other one with four radii defined (can be the same).

Besides satisfying my idle curiosity and honing my AOSP code diving skills, I have learned something useful: do not mix paths and round rects on Canvas and use Path.addRoundRect with radii array when your path contains other curved shapes.

Thneed, notes and db design

We're starting to find more and more interesting use cases for Thneed in Base CRM codebase. The first release using it, and few other libraries we recently developed, was released just before the Halloween and we haven't registered any critical issues related to it. All in all, the results look very promising. I won't recommend using Thneed in your production builds yet, but I urge you to star the project on Github and watch its progress.

The Thneed was created as an answer to some issues we faced when developing and maintaining Base CRM, and this fact is sometimes reflected by the API. The example of this is something we internally called PolyModels.

Let's start with a scenario, where we have a some objects and we'd like to add notes to. It's a classic one-to-many relationship, which I'd model with a foreign key in notes table:

CREATE TABLE some_entity (id INTEGER);
CREATE TABLE notes (
    id INTEGER, 
    some_entity_id INTEGER REFERENCES some_entity(id), 
    content TEXT
);

Now let's introduce another type of objects, which also can have notes attached to it. We have few options now. The simplest thing to do is to keep these notes in a completely separate table:

CREATE TABLE other_entity (id INTEGER);
CREATE TABLE other_enity_notes (
    id INTEGER, 
    other_entity_id INTEGER REFERENCES other_entity(id), 
    content TEXT
);

The issue with this solution is that we have two separate schemas that need to be updated in parallel, and in 95% of cases would be exactly the same. Another approach is making the objects which contain notes sort of inherit a base class:

CREATE TABLE notables (id INTEGER);
CREATE TABLE some_entity (id INTEGER, notable_id INTEGER REFERENCES notables(id));
CREATE TABLE other_entity (id INTEGER, notable_id INTEGER REFERENCES notables(id));
CREATE TABLE notes (
    id INTEGER, 
    notable_id INTEGER REFERENCES notables(id), 
    content TEXT
);

These two solutions work perfectly in the "give me all notes for object X" scenario, but it gets ugly if you want to display a single note with the simple "Associated with object X" info. In this case you have to query every model which can contain notes, to see if this particular association references the objects from this model. On top of that, the Noteable table approach requires some additional work to create the entry in

You can always have a several mutually exclusive foreing keys in your notes:

CREATE TABLE some_entity (id INTEGER);
CREATE TABLE other_entity (id INTEGER);
CREATE TABLE notes (
    id INTEGER, 
    some_entity_id INTEGER REFERENCES some_entity(id), 
    other_entity_id INTEGER REFERENCES other_entity(id), 
    content TEXT
);

But this solution doesn't really scale well as the number of the models which can contain notes increases. Also, your DBAs will love you if you go this way.

The solution to this problem we used in Base was to have two columns in Notes table: one holding the type of the "noteable" object, i.e. and the other for the id of this object:

CREATE TABLE some_entity (id INTEGER);
CREATE TABLE other_entity (id INTEGER);
CREATE TABLE notes (
    id INTEGER, 
    noteable_id INTEGER, 
    noteable_type TEXT, 
    content TEXT
);

The glaring issue with this approach is losing the consistency guarantee - no database I know of support this kind of foreign keys. But when you have SOA on the backend and the notes are stored in a separate database than the noteable objects, this is not your top concern. On mobile apps, even though we have a single database, we use the same structure, because all the other have some implementation issues and worse performance characteristics.

I'm not a db expert, and I haven't found any discussion of similar cases, which means that a) we're doing something very wrong or b) we have just very specific requirements. Let me know if it's a former case.

I needed to model this relationships in Thneed, which tured out to be quite tricky, but that's the topic for another blog post.

Mobilization 2013

I gave the talk at the Mobilization 2013 conference this weekend. I have presented few libraries created at Base CRM to make the data model maintainable. Here are the slides:


Besides giving the talk, I have attended few other very interesting talks: +Wojtek Erbetowski has shown the way they test Android apps at Polidea using RoboSpock; +Mateusz Grzechociński introduced Android devs to new build system and shared an awesome gradle protip: use --daemon command line parameter to shave off few seconds from gradle startup. +Mateusz Herych described the Dagger basics and warned about few pitfalls. Mieszko Lassota described some UI blunders not only from the programming world. Finally, Krzysztof Kocel and Paweł Urban summarised the security pitfalls.

All in all, this years Mobilization conference was a great place to be at. See you there next year!

More Guava goodies - AbstractIterator

A while ago I wanted to perform a certain operation for every subsequent pair of elements in collection, i.e. for list [1, 2, 3, 4, 5] I wanted to do something with pairs (1, 2), (2, 3), (3, 4), (4, 5). In Haskell that would be easy:

Prelude> let frobnicate list = zip (init list) (tail list)
Prelude> frobnicate [1..5]
[(1,2),(2,3),(3,4),(4,5)]

The problem is, I needed this stuff in my Android app, which means Java. The easiest thing to write would be obviously:

List<T> list;
for (int i = 1; i != list.size(); ++i) {
  T left = list.get(i-1);
  T right = list.get(i);

  // do something useful
}

But where's the fun with that? Fortunately, there is Guava. It doesn't have the zip or init functions, but it provides tool to write them yourself - the AbstractIterator. Tl;dr of the documentation: override one method returning an element or returning special marker from endOfData() method result.

The zip implementation is pretty straightforward:

public static <TLeft, TRight> Iterable<Pair<TLeft, TRight>> zip(final Iterable<TLeft> left, final Iterable<TRight> right) {
  return new Iterable<Pair<TLeft, TRight>>() {
    @Override
    public Iterator<Pair<TLeft, TRight>> iterator() {
      final Iterator<TLeft> leftIterator = left.iterator();
      final Iterator<TRight> rightIterator = right.iterator();

      return new AbstractIterator<Pair<TLeft, TRight>>() {
        @Override
        protected Pair<TLeft, TRight> computeNext() {
          if (leftIterator.hasNext() && rightIterator.hasNext()) {
            return Pair.create(leftIterator.next(), rightIterator.next());
          } else {
            return endOfData();
          }
        }
      };
    }
  };
}

The tail can be achieved simply by calling the Iterables.skip:

public static <T> Iterable<T> getTail(Iterable<T> iterable) {
  Preconditions.checkArgument(iterable.iterator().hasNext(), "Iterable cannot be empty");
  return Iterables.skip(iterable, 1);
}

For init you could write similar function:

public static <T> Iterable<T> getInit(final Iterable<T> iterable) {
  Preconditions.checkArgument(iterable.iterator().hasNext(), "Iterable cannot be empty");
  return Iterables.limit(iterable, Iterables.size(iterable));
}

But this will iterate through the entire iterable to count the size. We don't need the count however, we just need to know if there is another element in the iterable. Here is more efficient solution:

public static <T> Iterable<T> getInit(final Iterable<T> iterable) {
  Preconditions.checkArgument(iterable.iterator().hasNext(), "Iterable cannot be empty");

  return new Iterable<T>() {
    @Override
    public Iterator<T> iterator() {
      final Iterator<T> iterator = iterable.iterator();
      return new AbstractIterator<T>() {
        @Override
        protected T computeNext() {
          if (iterator.hasNext()) {
            T t = iterator.next();
            if (iterator.hasNext()) {
              return t;
            }
          }
          return endOfData();
        }
      };
    }
  };
}

All methods used together look like this:

List<T> list;
for (Pair<T, T> zipped : zip(getInit(list), getTail(list))) {
  // do something useful
}

Mobilization 2013 and Android Tech Talks meetup

I'll give the presentation on this years Mobiliztion conference in Łódź on October 26th:

I'll talk about challenges related to ContentProvider and data model in general we faced during 2 years of development of Base CRM for Android. Even if this particular topic does not concern you, the agenda is ripe with other interesting Android topics: dependency injection with Dagger, Gradle, unit testing, continuous integration. It's not Android specific event - there are also several presentations about other mobile platforms.

If you already have other plans for October 26th, you want to share some war stories related to data model on Android or you just want to talk about Android with fellow geeks, I recommend you a MeetUp happening next week in Kraków: Android Tech Talks #3. I'll give a short topic intro, which (I hope) will be followed by deep, insightful discussion.

Guava goodies

This is a long overdue post after my Guava on Android post from February. Since then I've been using Guava in pretty much every Java project I was involved in and I still find new stuff that makes my code both shorter and clearer. Some random examples:

Objects.equal()

// instead of:
boolean equal = one == null
    ? other == null
    : one.equals(other);

// Guava style:
boolean equal = Objects.equal(one, other);

Objects.hashcode()

// instead of:
@Override
public int hashCode() {
  int result = x;
  result = 31 * result + (y != null ? Arrays.hashCode(y) : 0);
  result = 31 * result + (z != null ? z.hashCode() : 0);
  return result;
}

// Guava style:
@Override
public int hashCode() {
  return Objects.hashCode(x, y, z);
}

Joiner

// instead of:
StringBuilder b = new StringBuilder();
for (int i = 0; i != a.length; ++i) {
  b.append(a[i]);
  if (i != a.length - 1) {
    b.append(", ");
  }
}
return b.toString();

// Guava style:
Joiner.on(", ").join(a);

ComparisonChain

// instead of:
@Override
public int compareTo(Person other) {
  int cmp = lastName.compareTo(other.lastName);
  if (cmp != 0) {
    return cmp;
  }
  cmp = firstName.compareTo(other.firstName);
  if (cmp != 0) {
    return cmp;
  }
  return Integer.compare(zipCode, other.zipCode);
}

// Guava style:
@Override
public int compareTo(Person other) {
  return ComparisonChain.start()
      .compare(lastName, other.lastName)
      .compare(firstName, other.firstName)
      .compare(zipCode, other.zipCode)
      .result();
}

Lists, Maps and Sets classes contain bunch of newFooCollection, which effectively replace the diamond operator from JDK7, but also allow you to initialize the collection from varargs.

Sets also contain the difference, intersection, etc. methods for common operations on sets, which a) have sane names, unlike some stuff from JDK's Collections, and b) doesn't change operands, so you don't have to make a defensive copy if you want to use the same set in two operations.

Speaking of defensive copying: Guava has a set of Immutable collections, which were created just for this purpose. There are few other very useful collections: LoadingCache, which you can think of as a lazy map with specified generator for new items; Multiset, very handy if you need to build something like a histogram; Table if you need to lookup value using two keys.

The other stuff I use very often are Preconditions. It's just a syntactic sugar for some sanity checks in your code, but it makes them more obvious, especially when you skim through some unfamiliar code. Bonus points: if you don't use the return values from checkNotNull and checkPositionIndex, you can remove those checks from performance critical sections using Proguard.

On Android you have the Log.getStackTraceString() helper method, but in plain Java you'd have to build one from Throwable.getStackTrace(). Only you don't have to do this, since Guava have Throwables.getStackTraceAsString() utility method.

Guava introduces also some functional idioms in form of Collections2.transform and Collections2.filter, but I have mixed feelings about them. On one hand sometimes they are life savers, but usually they make the code much uglier than the good ol' imperative for loop, so ues them with caution. They get especially ugly when you need to chain multiple transformations and filters, but for this case the Guava provides the FluentIterable interface.

None of the APIs listed above is absolutely necessary, but seriously, you want to use Guava (but sometimes not the latest version). Each part of it raises the abstraction level of your code a tiny bit, improving it one line at the time.