Skip to content

Latest commit

 

History

History
602 lines (477 loc) · 17.3 KB

File metadata and controls

602 lines (477 loc) · 17.3 KB

Chat 项目代码审查报告

审查时间: 2026-03-02
审查人: Linus 风格代码审查
项目: forgeekscn/chat
代码统计: 37 个 Java 文件, 30 个 XML 文件


总体评价

这代码写得像 2016 年的实习生项目。到处都是硬编码敏感信息、SQL 注入漏洞、不安全的网络通信。能跑不代表能上线。

严重程度分布:

  • 🔴 Critical(必须修复): 3 类 25+ 处问题
  • 🟡 Important(应该修复): 4 类 15+ 处问题
  • 🔵 Minor(建议修复): 3 类 20+ 处问题

🔴 Critical(必须修复)

1. 硬编码敏感信息(最严重!)

问题描述:
把 API key、token、服务器 IP 直接写在代码里?你是想让全世界都看到吗?反编译一下 APK 就全暴露了。

影响文件:

App.java (行 14)

public static String ip="51052e61.nat123.net";  // 服务器地址硬编码

LoginActivity.java (行 148, 215)

params.add("key", "10eaa1e59ceeb2cbcd1ac446da43e3a9");  // 聚合数据 key
CrashReport.initCrashReport(getApplicationContext(), "900023713", true, userStrategy);  // Bugly AppID
// 注释掉的代码里还有:
// params.put("apikey", "34ca0f7732b7f0718ad418e3e7d6ed08");  // 云片短信 API key

Register1.java (行 92)

params.put("apikey", "34ca0f7732b7f0718ad418e3e7d6ed08");  // 云片短信 API key

StationData.java (行 44, 92)

params.add("key", "10eaa1e59ceeb2cbcd1ac446da43e3a9");  // 聚合数据 key(重复)

AndroidManifest.xml (行 60, 64, 68)

<meta-data android:name="com.baidu.lbsapi.API_KEY" 
           android:value="3a0vVIWyrZC2CQSdrvvObnM7123OugOG" />  <!-- 百度地图 key -->
<meta-data android:name="com.thinkland.juheapi.openid"
           android:value="JH118022b1118eae05e616a68b0d7354c3" />  <!-- 聚合 openid -->

修复方案:

  1. 所有敏感信息移到 gradle.propertieslocal.properties
  2. 使用 BuildConfig 在编译时注入
  3. 服务器地址使用配置文件或远程下发
  4. API key 使用 NDK 加密存储(至少增加破解难度)

示例修复:

// gradle.properties
API_KEY_JUHE=10eaa1e59ceeb2cbcd1ac446da43e3a9
API_KEY_BAIDU=3a0vVIWyrZC2CQSdrvvObnM7123OugOG
SERVER_IP=51052e61.nat123.net
// app/build.gradle
android {
    defaultConfig {
        buildConfigField "String", "API_KEY_JUHE", "\"${API_KEY_JUHE}\""
        buildConfigField "String", "SERVER_IP", "\"${SERVER_IP}\""
    }
}
// 使用
String url = "http://" + BuildConfig.SERVER_IP + "/chat/login.php";

2. SQL 注入风险(致命漏洞!)

问题描述:
直接把用户输入拼接到 URL 里?你是嫌数据库不够危险吗?任何一个高中生都能通过 URL 注入 DROP TABLE。

影响文件:

LoginActivity.java (行 261)

String url = "http://" + App.ip + "/chat/login.php?username=" + 
    username.getText().toString() + "&password=" + password.getText().toString();

攻击示例: username=admin' OR '1'='1 → 绕过登录

Register1.java (行 79)

String url = "http://" + App.ip + "/chat/reg.php?username=" + 
    username.getText().toString() + "&password=" + password.getText().toString();

AddFriendActivity.java (行 41)

String url = "http://" + App.ip + "/chat/addFriend1.php?username=" + 
    App.username + "&target=" + friendName.getText().toString();

FriendListActivity.java (行 75, 43)

String url = "http://" + App.ip + "/chat/getFriendList.php?username=" + App.username;
String url = "http://" + App.ip + "/chat/getname.php?username=" + string;

EditProfile.java (行 69)

params.add("username", App.username);  // 虽然用了 POST,但服务端可能仍然不安全

Myprofile.java (行 50)

String url = "http://" + App.ip + "/chat/getAllUserInfor.php?username=" + App.username;

main.java (未完全列出,但同样存在大量拼接)

String url = "http://" + App.ip + "/chat/onlinelist.php?username=" + 
    App.username + "&x=" + App.x + "&y=" + App.y;

修复方案:

  1. 所有网络请求必须使用 POST + RequestParams(AsyncHttpClient 已支持)
  2. 服务端必须使用 PreparedStatement 或 ORM 框架
  3. 客户端进行输入验证(长度、特殊字符过滤)
  4. 重要数据加密传输

示例修复:

// 错误示范 ❌
String url = "http://" + App.ip + "/chat/login.php?username=" + username + "&password=" + password;
client.get(url, handler);

// 正确示范 ✅
String url = "http://" + BuildConfig.SERVER_IP + "/chat/login.php";
RequestParams params = new RequestParams();
params.put("username", username.getText().toString().trim());
params.put("password", password.getText().toString().trim());
client.post(url, params, handler);

// 输入验证
private boolean validateInput(String username, String password) {
    if (username == null || username.length() < 3 || username.length() > 20) {
        Toast.makeText(this, "用户名长度必须在 3-20 之间", Toast.LENGTH_SHORT).show();
        return false;
    }
    if (password == null || password.length() < 6) {
        Toast.makeText(this, "密码长度必须至少 6 位", Toast.LENGTH_SHORT).show();
        return false;
    }
    // 过滤特殊字符
    if (username.matches(".*[;'\"\\\\].*")) {
        Toast.makeText(this, "用户名包含非法字符", Toast.LENGTH_SHORT).show();
        return false;
    }
    return true;
}

3. 不安全的网络通信

问题描述:
全部使用 HTTP 明文传输?用户密码、token、个人信息全部裸奔?中间人攻击者笑醒了。

影响范围:
所有网络请求(25+ 处)都使用 HTTP

修复方案:

  1. 全部迁移到 HTTPS
  2. 使用证书锁定(Certificate Pinning) 防止中间人攻击
  3. 敏感数据(密码)客户端加密后再传输
  4. 添加网络请求超时和重试机制

示例修复:

// 错误 ❌
String url = "http://" + App.ip + "/chat/login.php";

// 正确 ✅
String url = "https://" + BuildConfig.SERVER_IP + "/chat/login.php";

// 更好的方案:使用 OkHttp + Certificate Pinning
OkHttpClient client = new OkHttpClient.Builder()
    .certificatePinner(new CertificatePinner.Builder()
        .add(BuildConfig.SERVER_IP, "sha256/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")
        .build())
    .connectTimeout(30, TimeUnit.SECONDS)
    .build();

🟡 Important(应该修复)

1. 过时的依赖和 API

问题描述:
这是 2016 年的项目吗?Gradle 1.0.0、compileSdkVersion 23、ButterKnife 5.1.1?Google 都不支持了你还用?

影响文件:

build.gradle

classpath 'com.android.tools.build:gradle:1.0.0'  // 2013 年的版本!

app/build.gradle

compileSdkVersion 23  // Android 6.0 (2015)
buildToolsVersion "21.1.2"  // 更老
targetSdkVersion 23

// 过时的依赖
compile 'com.jakewharton:butterknife:5.1.1'  // 应该用 8.8.1 或迁移到 ViewBinding
compile 'cz.msebera.android:httpclient:4.3.6'  // Apache HttpClient 早就废弃了
compile files('libs/android-async-http-1.4.9.jar')  // 2016 年停止维护

修复方案:

  1. Gradle 升级到 7.0+
  2. compileSdkVersion 升级到 33+ (Android 13)
  3. targetSdkVersion 升级到 33+(Google Play 要求)
  4. ButterKnife 迁移到 ViewBinding
  5. AsyncHttpClient 迁移到 OkHttp + Retrofit
  6. Support Library 迁移到 AndroidX

示例修复:

// build.gradle (Project)
dependencies {
    classpath 'com.android.tools.build:gradle:7.4.2'
}

// app/build.gradle
android {
    compileSdkVersion 33
    buildToolsVersion "33.0.1"
    
    defaultConfig {
        minSdkVersion 21
        targetSdkVersion 33
    }
    
    buildFeatures {
        viewBinding true  // 启用 ViewBinding
    }
}

dependencies {
    implementation 'androidx.appcompat:appcompat:1.6.1'
    implementation 'androidx.constraintlayout:constraintlayout:2.1.4'
    implementation 'com.squareup.okhttp3:okhttp:4.10.0'
    implementation 'com.squareup.retrofit2:retrofit:2.9.0'
    implementation 'com.squareup.retrofit2:converter-gson:2.9.0'
}
// ViewBinding 示例(替代 ButterKnife)
public class LoginActivity extends AppCompatActivity {
    private ActivityLoginBinding binding;  // 自动生成
    
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        binding = ActivityLoginBinding.inflate(getLayoutInflater());
        setContentView(binding.getRoot());
        
        // 使用
        binding.login.setOnClickListener(v -> login());
        binding.gotoRegister.setOnClickListener(v -> {
            startActivity(new Intent(this, RegisterActivity.class));
        });
    }
    
    @Override
    protected void onDestroy() {
        super.onDestroy();
        binding = null;  // 防止内存泄漏
    }
}

2. 内存泄漏风险

问题描述:
大量静态变量、未关闭的资源、Activity 上下文泄露。内存泄漏像筛子一样。

影响文件:

App.java (行 12-20)

public static String token = null;
public static String username = null;
public static boolean isLogin = false;
public static String ip = "51052e61.nat123.net";
public static double x = 0;
public static double y = 0;
public static boolean islogout = false;
public static SharedPreferences sharedPreferences = null;

问题: 全局静态变量会一直占用内存,SharedPreferences 持有 Application Context 还好,但其他数据应该用更好的方式管理。

LoginActivity.java (行 227)

AlertDialog.Builder builder = new AlertDialog.Builder(LoginActivity.this);
builder.setIcon(R.drawable.rc_progress_sending_style);
builder.setTitle("正在登陆");
builder.show();
// 没有保存引用,无法在 onDestroy 时关闭

BaiduMapActivity.java

public LocationClient mLocationClient = null;
// 在 onDestroy 中没有调用 mLocationClient.stop()

StationData.java (行 21)

Handler handler = new Handler() { ... }  // 非静态内部类会持有外部类引用

修复方案:

  1. 使用 ViewModel + LiveData 管理数据(避免静态变量)
  2. AlertDialog 在 Activity 销毁时必须 dismiss
  3. Handler 使用静态内部类 + WeakReference
  4. 资源(LocationClient、AsyncHttpClient)在 onDestroy 时释放

示例修复:

// 错误 ❌
public class App extends Application {
    public static String token = null;
    public static String username = null;
}

// 正确 ✅ - 使用 ViewModel
public class UserViewModel extends AndroidViewModel {
    private MutableLiveData<String> token = new MutableLiveData<>();
    private MutableLiveData<String> username = new MutableLiveData<>();
    
    public LiveData<String> getToken() { return token; }
    public void setToken(String value) { token.setValue(value); }
}

// Handler 内存泄漏修复
// 错误 ❌
Handler handler = new Handler() {
    public void handleMessage(Message msg) { ... }
};

// 正确 ✅
private static class SafeHandler extends Handler {
    private final WeakReference<StationData> outerClass;
    
    SafeHandler(StationData outer) {
        outerClass = new WeakReference<>(outer);
    }
    
    @Override
    public void handleMessage(Message msg) {
        StationData data = outerClass.get();
        if (data != null) {
            // 处理消息
        }
    }
}

// AlertDialog 修复
public class LoginActivity extends AppCompatActivity {
    private AlertDialog progressDialog;
    
    @Override
    protected void onDestroy() {
        if (progressDialog != null && progressDialog.isShowing()) {
            progressDialog.dismiss();
        }
        super.onDestroy();
    }
    
    private void showProgress() {
        progressDialog = new AlertDialog.Builder(this)
            .setTitle("正在登陆")
            .setIcon(R.drawable.rc_progress_sending_style)
            .create();
        progressDialog.show();
    }
}

3. 缺少输入验证

问题描述:
用户输入什么你都敢直接用?长度、格式、特殊字符都不检查?

影响文件:
几乎所有包含 EditText 的 Activity

修复方案:
见上文 SQL 注入部分的示例


4. 缺少错误处理和日志安全

问题描述:
到处都是 Log.e("hechao", ...),生产环境会把敏感信息打印出来。异常捕获后只是 e.printStackTrace(),用户什么都不知道。

影响文件:
几乎所有 Java 文件

修复方案:

// 错误 ❌
Log.e("hechao", "token: " + token);  // 生产环境会暴露 token
try {
    ...
} catch (JSONException e) {
    e.printStackTrace();  // 用户看不到,开发者也看不到
}

// 正确 ✅
if (BuildConfig.DEBUG) {
    Log.d("LoginActivity", "Login successful");  // 只在 debug 模式打印
}

try {
    ...
} catch (JSONException e) {
    Log.e("LoginActivity", "Failed to parse login response", e);
    if (BuildConfig.DEBUG) {
        e.printStackTrace();
    }
    runOnUiThread(() -> 
        Toast.makeText(this, "登录失败,请重试", Toast.LENGTH_SHORT).show()
    );
}

🔵 Minor(建议修复)

1. 代码规范问题

问题列表:

  • ❌ 类名小写开头:main.java, Myrun.java → 应该是 MainActivity, MyRunActivity
  • ❌ 大量注释掉的代码(应该删除,git 会保留历史)
  • ❌ 魔法数字:option.setScanSpan(1000) → 应该定义常量
  • ❌ 字符串硬编码:"正在登陆", "network error" → 应该用 strings.xml
  • ❌ 缺少注释和文档

示例修复:

// 错误 ❌
public class main extends Activity { ... }
option.setScanSpan(1000);
Toast.makeText(this, "正在登陆", Toast.LENGTH_SHORT).show();

// 正确 ✅
public class MainActivity extends AppCompatActivity { ... }

// 常量定义
private static final int LOCATION_UPDATE_INTERVAL_MS = 1000;
option.setScanSpan(LOCATION_UPDATE_INTERVAL_MS);

// strings.xml
<string name="login_progress">正在登陆...</string>
<string name="error_network">网络错误请检查网络连接</string>

// 使用
Toast.makeText(this, R.string.login_progress, Toast.LENGTH_SHORT).show();

2. 性能优化

问题列表:

  • ❌ 在主线程进行网络请求(虽然 AsyncHttpClient 是异步的,但应该用更现代的方案)
  • ❌ Bitmap 未回收(BitmapFactory.decodeByteArray 后应该回收)
  • ❌ ListView 未使用 ViewHolder 模式(MyAdapter

示例修复:

// Bitmap 回收
Bitmap bitmap = BitmapFactory.decodeByteArray(bytes, 0, bytes.length);
photo123.setImageBitmap(bitmap);
// 在 onDestroy 或 ImageView 不再使用时
if (bitmap != null && !bitmap.isRecycled()) {
    bitmap.recycle();
}

// ViewHolder 模式
public class MyAdapter extends BaseAdapter {
    static class ViewHolder {
        TextView textView;
    }
    
    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        ViewHolder holder;
        if (convertView == null) {
            convertView = LayoutInflater.from(context).inflate(R.layout.item_friend, parent, false);
            holder = new ViewHolder();
            holder.textView = convertView.findViewById(R.id.text);
            convertView.setTag(holder);
        } else {
            holder = (ViewHolder) convertView.getTag();
        }
        holder.textView.setText(friendList.get(position));
        return convertView;
    }
}

3. 用户体验

问题列表:

  • ❌ 网络请求没有 loading 状态
  • ❌ 错误提示不友好("network error" → 应该告诉用户具体原因)
  • ❌ 没有空状态处理(好友列表为空时应该显示提示)
  • ❌ 没有网络状态检测

📊 修复优先级

优先级 类别 预计工作量 风险
P0 硬编码敏感信息 2 小时 🔴 极高(数据泄露)
P0 SQL 注入 4 小时 🔴 极高(数据被删)
P0 HTTP 明文传输 3 小时 🔴 极高(中间人攻击)
P1 过时依赖升级 8 小时 🟡 中等(兼容性问题)
P1 内存泄漏 4 小时 🟡 中等(应用崩溃)
P2 代码规范 6 小时 🟢 低(维护成本)
P2 性能优化 4 小时 🟢 低(用户体验)

总工作量: 约 31 小时(4 个工作日)


🎯 下一步行动

立即执行(本次重构)

  1. ✅ 移除所有硬编码敏感信息到 BuildConfig
  2. ✅ 修复 SQL 注入(使用 POST + RequestParams)
  3. ✅ 添加输入验证
  4. ✅ 移除注释掉的代码
  5. ✅ 修复内存泄漏(AlertDialog、Handler)

短期规划(1 周内)

  1. ⏳ 迁移到 HTTPS(需要后端配合)
  2. ⏳ 升级 Gradle 和依赖库
  3. ⏳ ButterKnife → ViewBinding

长期规划(1 月内)

  1. ⏳ 重构网络层(Retrofit + OkHttp)
  2. ⏳ 引入架构组件(ViewModel + LiveData)
  3. ⏳ 完善错误处理和日志系统

💬 Linus 点评

"这代码写得像是没人 code review 过。硬编码密钥、SQL 注入、HTTP 明文传输,2016 年的安全事故大礼包。能跑不代表能上线,能上线不代表不会被人搞。"

"我知道这是学习项目,但安全意识要从第一行代码就开始培养。现在不修,以后到了生产环境就是灾难。"

"Talk is cheap. Show me the fix."


报告生成时间: 2026-03-02 22:15
下一步: 开始重构代码