Merge changes from topic "tp-master-security"
* changes:
Remove APN db reading loophole
Revert "DO NOT MERGE Check access to user and password fields in APN db"
Revert "DO NOT MERGE Examine sort field for sensitive fields"
Revert "DO NOT MERGE Fix the remaining holes for access to APN data"
DO NOT MERGE Fix the remaining holes for access to APN data
diff --git a/src/com/android/providers/telephony/SqlTokenFinder.java b/src/com/android/providers/telephony/SqlTokenFinder.java
deleted file mode 100644
index 9be49fe..0000000
--- a/src/com/android/providers/telephony/SqlTokenFinder.java
+++ /dev/null
@@ -1,170 +0,0 @@
-/*
- * Copyright (C) 2019 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License
- */
-package com.android.providers.telephony;
-
-import android.annotation.Nullable;
-
-import java.util.function.Consumer;
-
-/**
- * Simple SQL parser to check statements for usage of prohibited/sensitive fields. Mostly copied
- * from
- * packages/providers/ContactsProvider/src/com/android/providers/contacts/sqlite/SqlChecker.java
- */
-public class SqlTokenFinder{
- private static boolean isAlpha(char ch) {
- return ('a' <= ch && ch <= 'z') || ('A' <= ch && ch <= 'Z') || (ch == '_');
- }
-
- private static boolean isNum(char ch) {
- return ('0' <= ch && ch <= '9');
- }
-
- private static boolean isAlNum(char ch) {
- return isAlpha(ch) || isNum(ch);
- }
-
- private static boolean isAnyOf(char ch, String set) {
- return set.indexOf(ch) >= 0;
- }
-
- private static char peek(String s, int index) {
- return index < s.length() ? s.charAt(index) : '\0';
- }
-
- /**
- * SQL Tokenizer specialized to extract tokens from SQL (snippets).
- *
- * Based on sqlite3GetToken() in tokenzie.c in SQLite.
- *
- * Source for v3.8.6 (which android uses): http://www.sqlite.org/src/artifact/ae45399d6252b4d7
- * (Latest source as of now: http://www.sqlite.org/src/artifact/78c8085bc7af1922)
- *
- * Also draft spec: http://www.sqlite.org/draft/tokenreq.html
- */
- public static void findTokens(@Nullable String sql, Consumer<String> checker) {
- if (sql == null) {
- return;
- }
- int pos = 0;
- final int len = sql.length();
- while (pos < len) {
- final char ch = peek(sql, pos);
-
- // Regular token.
- if (isAlpha(ch)) {
- final int start = pos;
- pos++;
- while (isAlNum(peek(sql, pos))) {
- pos++;
- }
- final int end = pos;
-
- final String token = sql.substring(start, end);
- checker.accept(token);
-
- continue;
- }
-
- // Handle quoted tokens
- if (isAnyOf(ch, "'\"`")) {
- final int quoteStart = pos;
- pos++;
-
- for (;;) {
- pos = sql.indexOf(ch, pos);
- if (pos < 0) {
- throw new IllegalArgumentException("Unterminated quote in" + sql);
- }
- if (peek(sql, pos + 1) != ch) {
- break;
- }
- // Quoted quote char -- e.g. "abc""def" is a single string.
- pos += 2;
- }
- final int quoteEnd = pos;
- pos++;
-
- if (ch != '\'') {
- // Extract the token
- final String tokenUnquoted = sql.substring(quoteStart + 1, quoteEnd);
-
- final String token;
-
- // Unquote if needed. i.e. "aa""bb" -> aa"bb
- if (tokenUnquoted.indexOf(ch) >= 0) {
- token = tokenUnquoted.replaceAll(
- String.valueOf(ch) + ch, String.valueOf(ch));
- } else {
- token = tokenUnquoted;
- }
- checker.accept(token);
- }
- continue;
- }
- // Handle tokens enclosed in [...]
- if (ch == '[') {
- final int quoteStart = pos;
- pos++;
-
- pos = sql.indexOf(']', pos);
- if (pos < 0) {
- throw new IllegalArgumentException("Unterminated quote in" + sql);
- }
- final int quoteEnd = pos;
- pos++;
-
- final String token = sql.substring(quoteStart + 1, quoteEnd);
-
- checker.accept(token);
- continue;
- }
-
- // Detect comments.
- if (ch == '-' && peek(sql, pos + 1) == '-') {
- pos += 2;
- pos = sql.indexOf('\n', pos);
- if (pos < 0) {
- // We disallow strings ending in an inline comment.
- throw new IllegalArgumentException("Unterminated comment in" + sql);
- }
- pos++;
-
- continue;
- }
- if (ch == '/' && peek(sql, pos + 1) == '*') {
- pos += 2;
- pos = sql.indexOf("*/", pos);
- if (pos < 0) {
- throw new IllegalArgumentException("Unterminated comment in" + sql);
- }
- pos += 2;
-
- continue;
- }
-
- // Semicolon is never allowed.
- if (ch == ';') {
- throw new IllegalArgumentException("Semicolon is not allowed in " + sql);
- }
-
- // For this purpose, we can simply ignore other characters.
- // (Note it doesn't handle the X'' literal properly and reports this X as a token,
- // but that should be fine...)
- pos++;
- }
- }
-}
diff --git a/src/com/android/providers/telephony/TelephonyProvider.java b/src/com/android/providers/telephony/TelephonyProvider.java
index b2d5487..0edbffc 100644
--- a/src/com/android/providers/telephony/TelephonyProvider.java
+++ b/src/com/android/providers/telephony/TelephonyProvider.java
@@ -106,7 +106,6 @@
import android.telephony.TelephonyManager;
import android.telephony.data.ApnSetting;
import android.text.TextUtils;
-import android.util.EventLog;
import android.util.Log;
import android.util.Pair;
import android.util.Xml;
@@ -2803,7 +2802,7 @@
List<String> constraints = new ArrayList<String>();
int match = s_urlMatcher.match(url);
- checkQueryPermission(match, projectionIn, selection, sort);
+ checkPermission();
switch (match) {
case URL_TELEPHONY_USING_SUBID: {
subIdString = url.getLastPathSegment();
@@ -3012,68 +3011,6 @@
return ret;
}
- private void checkQueryPermission(int match, String[] projectionIn, String selection,
- String sort) {
- if (match != URL_SIMINFO && match != URL_SIMINFO_USING_SUBID) {
- // Determine if we need to do a check for fields in the selection
- boolean selectionOrSortContainsSensitiveFields;
- try {
- selectionOrSortContainsSensitiveFields = containsSensitiveFields(selection);
- selectionOrSortContainsSensitiveFields |= containsSensitiveFields(sort);
- } catch (IllegalArgumentException e) {
- // Malformed sql, check permission anyway and return.
- checkPermission();
- return;
- }
-
- if (selectionOrSortContainsSensitiveFields) {
- try {
- checkPermission();
- } catch (SecurityException e) {
- EventLog.writeEvent(0x534e4554, "124107808", Binder.getCallingUid());
- throw e;
- }
- }
-
- if (projectionIn != null) {
- for (String column : projectionIn) {
- if (TYPE.equals(column) ||
- MMSC.equals(column) ||
- MMSPROXY.equals(column) ||
- MMSPORT.equals(column) ||
- MVNO_TYPE.equals(column) ||
- MVNO_MATCH_DATA.equals(column) ||
- APN.equals(column)) {
- } else {
- checkPermission();
- break;
- }
- }
- } else {
- // null returns all columns, so need permission check
- checkPermission();
- }
- } else {
- // if querying siminfo, caller should have read privilege permissions
- checkPhonePrivilegePermission();
- }
- }
-
- private boolean containsSensitiveFields(String sqlStatement) {
- try {
- SqlTokenFinder.findTokens(sqlStatement, s -> {
- switch (s) {
- case USER:
- case PASSWORD:
- throw new SecurityException();
- }
- });
- } catch (SecurityException e) {
- return true;
- }
- return false;
- }
-
/**
* To find the current sim APN. Query APN based on {MCC, MNC, MVNO} to support backward
* compatibility but will move to carrier id in the future.