一个埋藏9年的底层bug发现历程

简介: 项目背景:一个负责店铺招牌拍摄的项目,App客户端开发中遇到照片损坏问题,尤其是使用webp格式时。问题已存在一段时间,最初采取临时措施让用户删除损坏照片,服务端也做损坏检测。排查过程包括:确认摄像头和图片压缩无误,发现加密解密流程不合理,修复了多余加密操作,但问题依旧存在。进一步分析发现,解密算法在处理某些特定图片时可能出现错误,原因是算法在处理非字符串数据时,错误地将空字符视为字符串结束标志。最终,修复了这个错误,成功解决了照片损坏问题。

项目背景

我所在的项目组主要负责对店铺招牌拍摄,我负责App客户端的开发工作。此项目从立项之初到现在已经有很长的历史了。

现在出现了一个问题:用户在拍摄照片时,会出现照片损坏的情况,这个问题在线上环境出现了有一段时间了,再加上自己接手时,此问题已经出现了,就没有深入排查过产生原因。暂时的解决策略是让用户手动删除损坏的照片,上传图片时,服务端也会进行一次文件损坏检测。

我们会下发各种拍摄任务类型,有的任务只需要拍摄几张照片即可,有的任务需要拍摄上千张图片,此问题就会更容易暴露。在同事的建议下,决定要找到问题的根源。

现象

之前只是知道有此问题,没有仔细研究过。经过自测+了解,初步明确了以下现象:

现象1:不同任务类型都有此问题

目前项目内的不同任务类型都共用同一个拍照存储模块。此现象可以明确,出错范围是在底层拍照存储模块,而不是在上层的业务逻辑。

现象2:1/200的概率稳定出现图片损坏

通过与同事的共同复现,发现连续拍摄200多张的时候,就会出现一张损坏的图片。这中间我们复现好多次,出现频率都很符合预期,甚至有一丝诡异,因为这个bug出现频率太稳定了,反而有些不正常了。面对此现象,当时想到了2种可能的情况:

  1. 概率和1/256(16进制的FF转为十进制的值,2的8次方,一字节[Byte]的大小)很接近,是不是由于在解析到某一字节时,出现了异常。
  2. 每拍摄200多张,此时就出现 重GC+手机温度过高导致降频,导致了卡顿,造成某一步执行超时或者失败。

以上只是猜测,完全没有任何证据,只是当时的思考方向。

现象3:仅webp格式会出现此问题

目前拍摄的图片有两种存储格式,分别是jpeg和webp格式。项目之前都是使用JPEG作为存储格式,后来为了减小图片的大小,开始改用webp格式进行存储。当我们把存储格式改为jpeg时,此问题不会出现;换为webp格式时,就是出现此问题。

统计了这两者的整体耗时(从图片字节流到存储到文件中),webp的用时大概是jpeg耗时的5倍;jpeg的存储大小是webp大小的1.5倍左右。面对此现象,当时的想法是处理图片耗时久,因而导致锁(线程锁、IO锁)竞争激烈,某一瞬间发生了数据冲突。

排查过程

首先熟悉了一下项目代码,下面是整个存储过程的流程图:

image.png


整个流程还是比较简单易懂的,按照我当时的怀疑方向,制定了以下排查顺序:

  1. 摄像头生成webp图片时出错了。
  2. 代码调用逻辑出错。
  3. 加密算法本身就有问题。

排查方向1:压制照片时出错

摄像头输出的图片在压制为webp照片的时候,就出现损坏了,而jpeg压制时不会损坏。该问题排查比较简单,只需要把未加密的原始webp图片也存储下来,与加密后无法解密的图片进行对照即可。实践之后,发现损坏的加密图片,对应的原始webp照片都是可以正常展示的

因此可以明确排除手机摄像头和压制webp图片的问题。

排查方向2:加密流程产生问题

调用AES加密算法的时候,调用可能会出错。比如:由于偶然情况,同一个图片被连续调用了两次加密算法。要排查此问题,需要深入阅读此部分的代码,并进行梳理。

先查阅了AES加密算法的相关资料。

AES是高级加密标准,在密码学中又称Rijndael加密法,是美国联邦政府采用的一种区块加密标准。这个标准用来替代原先的DES,目前已经被全世界广泛使用,同时AES已经成为对称密钥加密中最流行的算法之一。AES支持三种长度的密钥:128位,192位,256位。

image.png

自己总结了一下:

  1. AES算法属于对称加密,加密和解密只需要一个相同的密钥;
  2. AES算法在对明文加密的时候,并不是把整个明文一股脑加密成一整段密文,而是把明文拆分成一个个独立的明文块,每一个明文块长度128bit;
  3. 在没有填充的情况下,密文和原文长度相等。

先重点看了一下线程安全问题,排查一圈,认真看了在此过程中所有涉及的共享变量,没有发现任何问题。

下面梳理了加密解密流程,发现了一个很严重的问题。此问题发生在预览图片部分,代码如下:

public Bitmap decode(ImageDecodingInfo decodingInfo) throws IOException {
    // 如果是本地图片,OriginalImageUri会是'file:///xxx'以此来判断是否正在加载本地图片
    boolean isLoaclFile = decodingInfo.getOriginalImageUri().startsWith("file://");
    String imagePath = null;
    if (isLoaclFile) {
        imagePath = decodingInfo.getImageUri().replaceFirst("file://", "");
        if (!TextUtils.isEmpty(imagePath) && new File(imagePath).exists()) {
            ImageEncryptTool.decrypt(imagePath);
        } else {
            return null;
        }
    }
    Bitmap bitmap = super.decode(decodingInfo);
    if (isLoaclFile) {
        ImageEncryptTool.encrypt(imagePath);
    }
    return bitmap;
}
// 解密代码
public static void encrypt(String filePath) throws IOException {
    try {
        RandomAccessFile raf = new RandomAccessFile(file, "rw");
        byte[] buffer = new byte[ENCRYPTED_SIZE];
        raf.read(buffer);
        buffer = JniArithmetic.aesEncryptNoPadding(buffer);
        raf.seek(0);
        raf.write(buffer);
        raf.close();
    } catch (IOException e) {
        e.printStackTrace();
        throw e;
    }
}

手机预览图片时,需要从手机磁盘中读取照片,进行解密后,转为bitmap的方式显示在屏幕上。上面这段代码的流程如下:

image.png

这里的逻辑很不合理,先把磁盘文件读取到内存解密,解密后再写回磁盘,此时磁盘上的图片是一个解密后的数据,再交由图片加载框架加载此图片,加载完成之后再进行加密,加密完再次写回文件。

此过程需要多次IO操作,执行效率很低。此过程不能保证是“原子性”操作,在流程中,发生任何问题都会导致最终的图片发生损坏,比如解密完成之后,由于崩溃导致没有进行加密。不合理的方式大大增加了出错概率。

另外,还有一个更严重的问题,当解密文件覆盖原文件后,另外一个线程可能会调用此文件,会把已经解密后的文件,再一次进行解密,解密完之后再重新覆盖写回,最终文件就是“一团乱麻”,会造成图片损坏。

修改为如下代码:

@Override
protected InputStream getImageStream(ImageDecodingInfo decodingInfo) throws IOException {
    // 如果是本地图片,OriginalImageUri会是'file:///xxx'以此来判断是否正在加载本地图片
    boolean isLoaclFile = decodingInfo.getOriginalImageUri().startsWith("file://");
    if (!isLoaclFile) {
        return super.getImageStream(decodingInfo);
    }
    // 解密本地图片
    InputStream imageStream = super.getImageStream(decodingInfo);
    byte[] encodeByteArray = inputStreamToByteArray(imageStream);
    final int ENCRYPTED_SIZE = 1024;
    byte[] decodeBuffer = new byte[ENCRYPTED_SIZE];
    System.arraycopy(encodeByteArray, 0, decodeBuffer, 0, ENCRYPTED_SIZE);
    decodeBuffer = JniArithmetic.aesDecryptNoPadding(decodeBuffer);
    System.arraycopy(decodeBuffer, 0, encodeByteArray, 0, ENCRYPTED_SIZE);
    return new ByteArrayInputStream(encodeByteArray);
}

在正确且合理的流程中,解密操作只会在内存中进行,不会写入到磁盘之中,这样就不会产生各种覆盖的情况了。

最后又排查了整个项目,移除了所有【解密再加密】 的过程,整个项目就保留了一处加密的地方,就是在第一次保存图片的时候,才会进行加密,然后再写入磁盘中。

成功解决?

这么明显的错误都被解决了,这时候想着,肯定没啥问题了。怀着十足的信心,进行了一番测试,可此时依然有此问题。刚开始有点不太确定,试了多次之后,可能100%确定问题依然未解决。

排查方向3:锁竞争问题

这时候又把视角转到了线程安全方向,为了使整个加密存储的耗时可控,我决定自己实现加解密算法。当然,我自己实现的加解密算法很简单。代码如下:

private static final int N=100;
public static byte[] aesEncrypt(byte[] data) {
    for (int i = 0; i < N; i++) {
        data[i] = (byte) (data[i] + 1);
    }
    return data;
}
public static byte[] aesDecrypt(byte[] data) {
    for (int i = 0; i < N; i++) {
        data[i] = (byte) (data[i] - 1);
    }
    return data;
}

只是简单地把每一个字节的值+1,解密的时候,再把每一个字节-1进行解密,我可以使用N的大小控制加解密耗时。又进行了一番测试,这时候不论怎么调整加解密耗时,都没有发生图片损坏的情况。

通过现有信息,加解密算法应该很有问题。但我依然相信加密算法没有问题,加密代码已经存在了9年之久,而且用的是很成熟的AES加密算法,应该不会有问题。

排查方向4:图片问题

从手机中把损坏的图片单独取出来,又分别用加密算法、解密算法处理这张图片。拿到了以下数据:

图1:未加密的原始照片,可以看到以RIFF开头,是用来识别webp文件的标志位

image.png

图2:按照代码流程加密结果

image.png

  • Case1:把原始图片,用加密算法单独跑一遍,发现内容和图2一致;
  • Case2:把加密图片,用解密算法单独运行一遍,发现内容和图1不一致,尝试多次后发现,每次解密的数据居然都是随机内容。

对应这一张图片,每次都把解密后的内容打印出来,发现有时候正确,有时候又是随机的,而且修改先后执行顺序,结果也可能不一样。由于加解密算法是由C++所写,根据以往经验,猜测出现此种情况,是由于C++内存残留导致的。

服务端在使用图片时,也需要进行解密,由于服务端不怕代码泄漏,因此直接使用了Java类库实现AES解密算法。在服务端同事的配合下,单独上传该图片,尝试了多次,发现服务端是可以稳定进行解密的。

又再服务端同事的帮助下,拿到了服务端解密算法,我把端上的解密算法替换为服务端解密算法,这张损坏的图片居然又可以正确展示了。最后又经过一番测试,发现再也没有出现图片损坏的情况。

到此,已经有95%的把握,可以证明是解密算法导致的。此时也可以安心下班了,第二天再排查解密算法。

排查方向5:AES解密算法

先向同事要到了加解密仓库的git地址,由于这块代码历史比较悠久,立项之初,使用SVN进行管理,后来迁移时整体被打包放到git仓库里,已经无法看到提交记录了。

项目本身的架构也比较老,使用了Android.mk作为构建工具,现在已经废弃很久了,我也没有接触过。在ChatGPT的帮助下,自动帮我转换成了现代的CMakeLists构建工具。接下来就可以正常的debug跟踪代码了。

AES算法本身就比较简单,只是不停地按照密码表,对原文进行替换。代码中没有使用任何三方库,自己实现了AES算法。

解密算法如下:

void AES::InvCipher( BYTE *input, BYTE *output, int len)
{
    int arrLen = len;
    unsigned char *uch_input = new unsigned char[arrLen + 1];
    strToUChar((const char*)input, uch_input, len);
    for (int i = 0; i < arrLen / 16; i++) {
        InvCipher(uch_input + i*16);
    }
    ucharToStr((const unsigned char*)uch_input, (char *)output, len);
    delete[] uch_input;
}
int AES::strToUChar(const char *ch, unsigned char *uch, int len) {
    int tmp = 0;
    if (ch == NULL || uch == NULL)
        return -1;
    if (strlen(ch) == 0)
        return -2;
    while (len) {
        tmp = (int) *ch;
        *uch++ = tmp;
        ch++;
        len--;
    }
    *uch = (int) '\0';
    return 0;
}

在跟踪到 if (strlen(ch) == 0) 这一行代码时,发现会直接返回-2作为错误码,上面的处理过程会直接忽视错误码,继续进行解密。当然,这时候肯定是无法解密成功的。

错误原因

我对这一做法进行了合理猜测,刚开始此处的加密只用于字符串,所以在入参时,会判断一下是否为空字符串。

在C++中,字符串通常以字符数组的形式表示,遵循C语言的传统。C/C++中的字符串是以空字符\0(ASCII值为0)结尾的字符数组。这种字符串被称为C-风格字符串或null-terminated字符串。判断字符串结束的方式就是检查每个字符,直到遇到\0。

char str[] = "hello";
// 字符串实际存储为 {'h', 'e', 'l', 'l', 'o', '\0'}

在JAVA中,字符串本身就存储了字符串的长度。这个长度字段在String对象创建时就被计算并存储起来,因此可以非常快速地调用length()方法来得到字符串的长度,而不需要遍历整个字符数组。

但是后来需要对二进制图片进行加密,二进制图片在存储过程中,也会用到'0x00'字节,但和字符串中的'0x00'含义完全不一样。如果二进制图片第一个字节为0x00,当成字符串处理就会得出加密内容为空,因而终止后续流程

又找了几张解密失败的图片,发现它们无一例外,开头第一个字节都是0x00,怪不得失败的概率是1/256。又把jpeg图片找来,所有jpeg图片的前16个字节(目测前500字节都是)是固定的,因此加密之后第一个字节就是固定。而webp格式加密后第一个字节是随机的,当然不会出现问题了。

问题解决

由于这部分代码已经存在了9年了,再加上自己对此部分代码不是很熟悉,秉承着最小改动的原则,只是去掉了对空字符串校验的情况,空字符可以提前校验。

int AES::strToUChar(const char *ch, unsigned char *uch, int len) {
    int tmp = 0;
    if (ch == NULL || uch == NULL)
        return -1;
//      if (strlen(ch) == 0)
//          return -2;
    while (len) {
        tmp = (int) *ch;
        *uch++ = tmp;
        ch++;
        len--;
    }
    *uch = (int) '\0';
    return 0;
}

后来在整理时,发现另外一个charToHex方法,居然有同样的代码,相同的两行已经被注释掉了,看来之前的人也发现了此问题,但没有把两个地方的相同问题一并解决了。

代码改动之后,又打包进行测试,发现再也没有出现此问题了。 最后能成功解决此问题,也是非常开心。

总结

从这件事情中,完美验证了“一个问题往往是由多个小的不规范或错误累积而成的”。每次的代码修改者,站在自身角度来看,没有造成大问题,单独来看,也不是完全不合理。

如果代码写得不规范,留有安全隐患,当时虽然不会暴露,所有风险问题汇总到一起的时候,就造成最后的“灾难”。

我也收获到了以下经验:

  1. 对于入参校验,应该提早进行校验,出现“非法入参”时,应当有合理的措施。比如:以返回值或者标志位的方式告诉调用者,实在不行可以造成崩溃,这样就能及早暴露问题。
  2. 底层模块要有通用性,不能只考虑当时的情况,此模块日后可能会在多种情况下使用;
  3. 要有风险意识,不要把风险问题扩大化,对同一份文件多次解密再加密,会出现错上加错的情况。
  4. 解决一个错误时,要看一下有没有相似的错误,可以一并修改。
目录
相关文章
|
1月前
|
算法 安全 数据安全/隐私保护
深入探究一个长期隐藏的底层bug的学习报告
在软件开发的过程中,底层bug往往像一颗定时炸弹,随时可能引发严重的问题。本文将分享我在开发过程中遇到的一个长期未被发现的底层bug,以及我如何逐步排查并最终解决这个问题的全过程。通过这次排查,我深刻认识到了代码规范性的重要性。一个不规范的代码修改,虽然短期内可能不会引起问题,但长期累积下来,可能会引发灾难性的后果。此外,我也意识到了底层模块的通用性和风险意识的重要性。在解决一个问题的同时,应该审视是否有相似的问题存在,以避免未来的风险。
89 3
|
1月前
|
存储 缓存 算法
作者推荐 | 【底层服务/编程功底系列】「底层技术原理」史上最清晰的采用程序员的视角方式进行深入探索Linux零拷贝技术原理及实现
作者推荐 | 【底层服务/编程功底系列】「底层技术原理」史上最清晰的采用程序员的视角方式进行深入探索Linux零拷贝技术原理及实现
36 0
|
1月前
|
存储 缓存 监控
【分布式技术专题】「缓存解决方案」一文带领你好好认识一下企业级别的缓存技术解决方案的运作原理和开发实战(场景问题分析+性能影响因素)
【分布式技术专题】「缓存解决方案」一文带领你好好认识一下企业级别的缓存技术解决方案的运作原理和开发实战(场景问题分析+性能影响因素)
56 0
|
1月前
|
NoSQL Java 程序员
阿里开发人员献礼“Java架构成长笔记”,深入内核,拒绝蒙圈
提起阿里,行外人联想到的关键词无非是“交易”、“淘宝”、“支付宝”,但对于程序员来说,阿里庞大的技术体系才是最吸引人的。实际上阿里作为国内一线互联网公司的头把交椅,内部的技术体系和发展都是备受关注的,对于程序员来说,能够进到阿里工作,就是对自己的技术水平进行一个提升和学习。
阿里开发人员献礼“Java架构成长笔记”,深入内核,拒绝蒙圈
|
8月前
|
程序员 开发者
|
7月前
|
SQL 安全 Java
硬核,腾讯内部整理的面向开发人员代码安全指南,适合所有程序员
硬核,腾讯内部整理的面向开发人员代码安全指南,适合所有程序员
56 0
|
11月前
|
测试技术 API
怎么在日常中提高你的编程找bug的能力
编程是一门需要细致入微和耐心的艺术。在编写代码的过程中,我们经常会遇到各种各样的错误和bug。因此,掌握一定的找bug能力对于提高代码质量和开发效率至关重要。本文将分享一些在日常中提高编程找bug能力的实用技巧。
100 0
|
11月前
|
设计模式 架构师 Java
程序员必修课:阿里性能优化全解终开源!设计+代码+JVM三飞
性能优化可以说是我们程序员的必修课,如果你想要跳出CRUD的苦海,成为一个更“高级”的程序员的话,性能优化这一关你是无论无何都要去面对的。为了提升系统性能,开发人员可以从系统的各个角度和层次对系统进行优化。除了最常见的代码优化外,在软件架构上、JVM虚拟机层、数据库以及操作系统层面都可以通过各种手段进行调优,从而在整体上提升系统的性能。
|
开发框架 JSON 缓存
从三层架构说起,谈谈对历史项目的小改造
任何框架无论封装的如何优秀,关键还是在于局中 “玩游戏” 的开发者,框架只是提供了基本的开发规则,要大家共同的遵循,这其中最难的就是团队小伙伴达成一致的思维认知和共识。约定由于配置,任何框架不可能面面俱到,过渡设计框架务必会失去部分灵活性(物极必反的道理想必大家都知...
199 1
从三层架构说起,谈谈对历史项目的小改造
|
设计模式 供应链 架构师
学习笔记 | 技术人成长的底层逻辑
学习笔记 | 技术人成长的底层逻辑
148 0