CycloneDX / cyclonedx-cocoapods

Creates CycloneDX Software Bill-of-Materials (SBOM) from Objective-C and Swift projects that use CocoaPods.
Apache License 2.0
21 stars 12 forks source link

crash with undefined method `empty?' for nil:NilClass #46

Closed nbersano closed 1 year ago

nbersano commented 1 year ago

https://github.com/CycloneDX/cyclonedx-cocoapods/blob/88a6212b3370250b23540c54d86a8ae8a5df8f0a/lib/cyclonedx/cocoapods/podfile_analyzer.rb#L116

In this line, the check is for an empty string, but if the key pod_name does not exist in the hash pods_cache is getting a null object and crashes.

Adding a check for pods_cache.key?(pod_name) fixed it for me.

cyclonedx-cocoapods version: 1.1.0 ruby version: 2.6 installation Method: https://github.com/CycloneDX/cyclonedx-cocoapods#from-source

macblazer commented 1 year ago

@nbersano Checking for the pod_name in the cache fixes the crash, but doesn't fix the underlying problem. The pods_cache is built from the Podfile.lock list of used Pods, and every pod in the pods_used array should be listed by name in the pods_cache.

The fact that you have a specific Podfile and Podfile.lock that seem to disagree means that there is some other logic problem in our code, probably around populating the cache from the lock file which happens in the simple_hash_of_lockfile_pods function (specifically lines 92-102 probably need some adjustment).

Would you be willing to include a copy of the PODS section of the Podfile.lock for the app that is causing problems?

nbersano commented 1 year ago

I was fearing something like that. So I refrained from submitting a PR.

I will check tomorrow morning if I can share the files, but, in the meantime, if it helps I remember it’s a dependency of a pod in the Podfile.lock file. But said dependency is not in our Podfile.

macblazer commented 1 year ago

No worries. We don't need the entire PODS section. If you can create a simple example of the problem where cyclonedx-cocoapods crashes, or even a good description of the problematic pod names and dependencies we can fabricate a test fixture to verify the fix. We currently have just a couple of fixtures, but the TestingPod is our example of bringing in a transitive dependency; MSAL/app-lib is not listed explicitly in the Podfile.

The Podfile defines what you want, and the lock file should have all of those plus any dependencies of those things even if not explicitly defined in the Podfile. In the code, the pods_used is from the Podfile and the pods_cache is from the lock file which is why everything in pods_used should be listed in pods_cache.

A few guesses are that maybe it's a local dependency, or something not from the standard CocoaPods spec repo, or maybe it's a pod with a non-standard naming convention. Do any of the pods have non-alphanumeric characters in the pod name (other than / for sub-specs)?

nbersano commented 1 year ago

@macblazer Hello, sorry for the late reply. I've narrowed it down to replicating the issue with only one pod in the Podfile.

I'm attaching the Podfile and the generated Podfile.lock

source 'https://github.com/CocoaPods/Specs.git'
platform :ios, '13.0'
use_frameworks!
def install_pods
  pod 'EFQRCode'
end

target 'cycloneDXcrash' do
  install_pods
end
PODS:
  - EFQRCode (6.2.1):
    - swift_qrcodejs (~> 2.2.2)

DEPENDENCIES:
  - EFQRCode

SPEC REPOS:
  https://github.com/CocoaPods/Specs.git:
    - EFQRCode

SPEC CHECKSUMS:
  EFQRCode: a4d39ec3466b68dffa71de3b5caef7c9ceefdc53

PODFILE CHECKSUM: 3e93ab9c0580591da64eca6661a70cd9abb23c67

COCOAPODS: 1.10.2

the crash happens when trying to get form the hash the key swift_qrcodejs

Note: This crash started to happen after updating cyclonedx-cocoapods to v1.1.0, previously we were using v 0.1.1 and it was working.

macblazer commented 1 year ago

Turns out that CocoaPods lists in the Podfile.lock all dependencies of a Pod, even if they're not actually used due to platform (or other) restrictions. EFQRCode has a dependency on swift_qrcodejs but only for watchOS; since your project is for iOS that watchOS dependency is not actually included. For a sample project that was for watchOS, swift_qrcodejs is included and cyclonedx-cocoapods works correctly.

I had not run into restricted dependencies in any of the projects previously, so this is a new case. I'll add a test fixture, write a test, and fix the code.

nbersano commented 1 year ago

sorry for the late reply. version 1.1.1 is not crashing anymore.

thanks.