arduino-cli lib install PacketSerial: nil pointer dereference

[r10r]

Environment

Rubens-MBP:arduino-cli ruben$ uname -a
Darwin Rubens-MBP.home 20.3.0 Darwin Kernel Version 20.3.0: Thu Jan 21 00:07:06 PST 2021; root:xnu-7195.81.3~1/RELEASE_X86_64 x86_64

Rubens-MBP:arduino ruben$ arduino-cli version
arduino-cli alpha Version: 0.15.1 Commit: c7403ed2 Date: 2021-02-04T10:11:39Z

Description

Hi devs, I tried to install the PacketSerial library which failed

Rubens-MBP:arduino ruben$ arduino-cli lib install PacketSerial
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x44be30c]

goroutine 1 [running]:
go.bug.st/relaxed-semver.(*Version).CompareTo(0xc000f2a630, 0x0, 0xc00171f5e9)
        /go/pkg/mod/go.bug.st/relaxed-semver@v0.0.0-20190922224835-391e10178d18/version.go:115 +0x3c
go.bug.st/relaxed-semver.(*Version).GreaterThan(...)
        /go/pkg/mod/go.bug.st/relaxed-semver@v0.0.0-20190922224835-391e10178d18/version.go:202
github.com/arduino/arduino-cli/arduino/libraries/librariesindex.(*Index).FindLibraryUpdate(0xc0001c2000, 0xc000581500, 0x0)
        /home/build/arduino/libraries/librariesindex/index.go:139 +0x88
github.com/arduino/arduino-cli/commands/lib.listLibraries(0xc00032c040, 0x4de0000, 0xc0014431d0, 0xc0037285a0, 0x0)
        /home/build/commands/lib/list.go:130 +0xe8
github.com/arduino/arduino-cli/commands/lib.LibraryResolveDependencies(0x4df1cc0, 0xc000038088, 0xc0014431d0, 0x1, 0xc0001c2dc8, 0x1)
        /home/build/commands/lib/resolve_deps.go:39 +0x145
github.com/arduino/arduino-cli/cli/lib.runInstallCommand(0xc001713340, 0xc0016bdaf0, 0x1, 0x1)
        /home/build/cli/lib/install.go:131 +0xed1
github.com/spf13/cobra.(*Command).execute(0xc001713340, 0xc0016bdad0, 0x1, 0x1, 0xc001713340, 0xc0016bdad0)
        /go/pkg/mod/github.com/spf13/cobra@v1.0.1-0.20200710201246-675ae5f5a98c/command.go:846 +0x29d
github.com/spf13/cobra.(*Command).ExecuteC(0xc0000bf8c0, 0x0, 0xc001699b00, 0x0)
        /go/pkg/mod/github.com/spf13/cobra@v1.0.1-0.20200710201246-675ae5f5a98c/command.go:950 +0x349
github.com/spf13/cobra.(*Command).Execute(...)
        /go/pkg/mod/github.com/spf13/cobra@v1.0.1-0.20200710201246-675ae5f5a98c/command.go:887
main.main()
        /home/build/main.go:31 +0x85

Analysis

This happens due to a missing nil check in FindLibraryUpdate for the following library:

&libraries.Library{Name:"UIPEthernet", Author:"ntruchsess", Maintainer:"Norbert Truchsess <norbert.truchsess@t-online.de>", Sentence:"Ethernet library for ENC28J60", Paragraph:"implements the same API as stock Ethernet-lib. Just replace the include of Ethernet.h with UIPEthernet.h", Website:"https://github.com/ntruchsess/arduino_uip", Category:"Uncategorized", Architectures:[]string{"avr"}, Types:[]string(nil), InstallDir:(*paths.Path)(0xc001797ad0), SourceDir:(*paths.Path)(0xc001797ad0), UtilityDir:(*paths.Path)(0xc001663930), Location:3, ContainerPlatform:(*cores.PlatformRelease)(nil), Layout:0x0, RealName:"UIPEthernet", DotALinkage:false, Precompiled:false, PrecompiledWithSources:false, LDflags:"", IsLegacy:false, Version:(*semver.Version)(nil), License:"Unspecified", Properties:(*properties.Map)(0xc0019f0750), Examples:paths.PathList{(*paths.Path)(0xc001663c10), (*paths.Path)(0xc001663c30), (*paths.Path)(0xc001663c60), (*paths.Path)(0xc001663c80), (*paths.Path)(0xc001663ca0), (*paths.Path)(0xc001663cc0)}, declaredHeaders:[]string(nil), sourceHeaders:[]string(nil), CompatibleWith:map[string]bool(nil)}

[silvanocerza]

I can't reproduce this, arduino-cli lib install PacketSerial installs the library correctly and without issues:

$ arduino-cli lib install PacketSerial
PacketSerial depends on PacketSerial@1.4.0
Downloading PacketSerial@1.4.0...
PacketSerial@1.4.0 already downloaded
Installing PacketSerial@1.4.0...
Already installed PacketSerial@1.4.0

Could you try and run that command again with the --verbose flag?

[r10r]

I tried to replay by bash history but unfortunately I can't reproduce it as well.
I've quickly added the nil check and ran the patched version which fixed it.

I did not look further than the patched method.
But there must be a case where the version is set to nil. Maybe a race condition?
Package data changed ?

I think the nil check doesn't hurt but it may hide the real error.
If I can reproduce it I'll attach the output of running the command with --verbose.
Is the whole state saved in /Library/Arduino15 on macOS ?

[per1234]

I do notice that the UIPEthernet library r10r has installed has an invalid version format:
https://github.com/ntruchsess/arduino_uip/blob/cf31df374b118d6e01513306ba36ac919cfb8fd9/library.properties#L8

version=1.04

(the leading zero on the 04).

Note that this is different from the UIPEthernet library available from Library Manager, which does have valid metadata. I can tell from the metadata that you don't have the Library Manager version installed.

I messed around with this a little bit and wasn't able to reproduce the panic though.

[r10r]

Ups I mistakenly used the patched version to reproduce this. Now with the unpatched version I can reproduce it.

Rubens-MBP:Library ruben$ ~/Downloads/arduino-cli_0.15.1_macOS_64bit/arduino-cli lib install --verbose PacketSerial
INFO[0000] arduino-cli version 0.15.1                   
INFO[0000] Checking signature                            index=/Users/ruben/Library/Arduino15/package_index.json signatureFile=/Users/ruben/Library/Arduino15/package_index.json.sig trusted=true
INFO[0000] Checking if CLI is Bundled into the IDE      
INFO[0000] Loading hardware from: /Users/ruben/Library/Arduino15/packages 
INFO[0000] Loading package arduino from: /Users/ruben/Library/Arduino15/packages/arduino/hardware 
INFO[0000] Checking signature                            error="opening signature file: open /Users/ruben/Library/Arduino15/packages/arduino/hardware/avr/1.8.3/installed.json.sig: no such file or directory" index=/Users/ruben/Library/Arduino15/packages/arduino/hardware/avr/1.8.3/installed.json signatureFile=/Users/ruben/Library/Arduino15/packages/arduino/hardware/avr/1.8.3/installed.json.sig
INFO[0000] Loaded platform                               platform="arduino:avr@1.8.3"
INFO[0000] Checking existence of 'tools' path: /Users/ruben/Library/Arduino15/packages/arduino/tools 
INFO[0000] Loading tools from dir: /Users/ruben/Library/Arduino15/packages/arduino/tools 
INFO[0000] Loaded tool                                   tool="arduino:arduinoOTA@1.3.0"
INFO[0000] Loaded tool                                   tool="arduino:avr-gcc@7.3.0-atmel3.6.1-arduino7"
INFO[0000] Loaded tool                                   tool="arduino:avrdude@6.3.0-arduino17"
INFO[0000] Loading package builtin from: /Users/ruben/Library/Arduino15/packages/builtin 
INFO[0000] Checking existence of 'tools' path: /Users/ruben/Library/Arduino15/packages/builtin/tools 
INFO[0000] Loading tools from dir: /Users/ruben/Library/Arduino15/packages/builtin/tools 
INFO[0000] Loaded tool                                   tool="builtin:ctags@5.8-arduino11"
INFO[0000] Loaded tool                                   tool="builtin:serial-discovery@1.1.0"
INFO[0000] Loading hardware from: /Users/ruben/Documents/Arduino/hardware 
INFO[0000] Loading package MySensors from: /Users/ruben/Documents/Arduino/hardware/MySensors 
INFO[0000] Loaded platform                               platform="MySensors:avr@"
INFO[0000] Adding libraries dir                          dir=/Users/ruben/Documents/Arduino/libraries location=user
INFO[0000] Adding libraries dir                          dir=/Users/ruben/Library/Arduino15/packages/arduino/hardware/avr/1.8.3/libraries location=platform
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x44be30c]

goroutine 1 [running]:
go.bug.st/relaxed-semver.(*Version).CompareTo(0xc000dba7e0, 0x0, 0xc0005aa369)
	/go/pkg/mod/go.bug.st/relaxed-semver@v0.0.0-20190922224835-391e10178d18/version.go:115 +0x3c
go.bug.st/relaxed-semver.(*Version).GreaterThan(...)
	/go/pkg/mod/go.bug.st/relaxed-semver@v0.0.0-20190922224835-391e10178d18/version.go:202
github.com/arduino/arduino-cli/arduino/libraries/librariesindex.(*Index).FindLibraryUpdate(0xc000120018, 0xc000582f00, 0x0)
	/home/build/arduino/libraries/librariesindex/index.go:139 +0x88
github.com/arduino/arduino-cli/commands/lib.listLibraries(0xc0017e3c00, 0x4de0000, 0xc003ad2320, 0xc0036da690, 0x0)
	/home/build/commands/lib/list.go:130 +0xe8
github.com/arduino/arduino-cli/commands/lib.LibraryResolveDependencies(0x4df1cc0, 0xc000124008, 0xc003ad2320, 0x2, 0xc000010988, 0x1)
	/home/build/commands/lib/resolve_deps.go:39 +0x145
github.com/arduino/arduino-cli/cli/lib.runInstallCommand(0xc001675340, 0xc000135b80, 0x1, 0x2)
	/home/build/cli/lib/install.go:131 +0xed1
github.com/spf13/cobra.(*Command).execute(0xc001675340, 0xc000135b60, 0x2, 0x2, 0xc001675340, 0xc000135b60)
	/go/pkg/mod/github.com/spf13/cobra@v1.0.1-0.20200710201246-675ae5f5a98c/command.go:846 +0x29d
github.com/spf13/cobra.(*Command).ExecuteC(0xc0001398c0, 0x0, 0xc0015f97a0, 0x0)
	/go/pkg/mod/github.com/spf13/cobra@v1.0.1-0.20200710201246-675ae5f5a98c/command.go:950 +0x349
github.com/spf13/cobra.(*Command).Execute(...)
	/go/pkg/mod/github.com/spf13/cobra@v1.0.1-0.20200710201246-675ae5f5a98c/command.go:887
main.main()
	/home/build/main.go:31 +0x85

[r10r]

@per1234 @silvanocerza Do you need more information ?

[silvanocerza]

Does lib list panic too? I think this might be caused by one of your installed library that has no version set in its library.properties, all the fields in that file are required as documented.

If there is actually a library that has no version would be great if you can find out which one it is.

Sorry for the late response, lots of things to do around here. :)

[r10r]

Does lib list panic too?

Yes it does. I've now added a logging statement back to index.FindLibraryUpdate and updated the pull request.

It's the library that I've pointed to initially and where @per1234 pointed to the malformed version

I do notice that the UIPEthernet library r10r has installed has an invalid version format, and printed out the `Library
https://github.com/ntruchsess/arduino_uip/blob/cf31df374b118d6e01513306ba36ac919cfb8fd9/library.properties#L8
version=1.04

I noticed that the library must have been installed previously by the arduino IDE.

InstallDir is set to /Users/ruben/Documents/Arduino/libraries/UIPEthernet whereas arduino-cli installs to /Users/ruben/Library/Arduino15 ?

Here is the zipped library folder /Users/ruben/Documents/Arduino/libraries/UIPEthernet UIPEthernet.zip

Output of lib list using the patched version. lib_list_patched.out.txt

[silvanocerza]

InstallDir is set to /Users/ruben/Documents/Arduino/libraries/UIPEthernet whereas arduino-cli installs to /Users/ruben/Library/Arduino15 ?

Nope, the CLI install libraries in /Users/ruben/Documents/Arduino, /Users/ruben/Library/Arduino15 is used to install cores/platforms and their tools. Respectively those folders are the values directories.user and directories.data in your config file.

Anyway. This is a peculiar issue because the error is caused by a library that's not respecting the documentation since it doesn't have a required property.

I think we should not even reach that point and avoid loading libraries that are not respecting the requirements, problem is that libraries under development, installed manually, using --git-url or --zip-path might not be loaded so this wouldn't be possible. What would you think @per1234 @ubidefeo? 🤔

I think your fix is the best way to go for now.

Again, sorry for the late response.

[per1234]

Here is the zipped library folder /Users/ruben/Documents/Arduino/libraries/UIPEthernet UIPEthernet.zip

That allowed me to reproduce the issue! I was not able to reproduce previously because I was installing the library into a folder named after the repository "arduino_uip". It turns out the error only occurs when the folder name matches the library.properties name value, which is the case with the .zip file you provided.

What would you think @per1234 @ubidefeo?

I don't think we should go out of our way to provide support for non-compliant projects, but that we should also accept their occurrence as an inevitability. In this particular case, UIPEthernet is a very popular library and the version issue was even fixed in a branch 7 years ago, but the library author never made that branch default and seems to have abandoned the library.

In Arduino Lint, we now have a dedicated tool for encouraging specification compliance, so it's reasonable to make Arduino CLI resilient to a hostile environment when doing so doesn't have a significantly harmful effect on the maintainability or functionality.

When the transistion was made to arduino-builder back in 2015, a lot of specification enforcement measures were added with the philosophy "the users will complain to the developers and the developers will fix the issues". But this really punished the users for problems that weren't in their code and that they didn't really have control over. The target user of the Arduino IDE just wants to get that LED blinking, not hunt down the cause of a cryptic error, submit a bug report, then wait years for a fix. Of course, the target user of Arduino CLI is different, but Arduino IDE is increasingly using more of Arduino CLI under the hood, so those users also will become unknowing Arduino CLI users.

And certainly it's important to effectively communicate what the problem is to the user when recovering from it is not possible. It would be very difficult for even the target Arduino CLI user to troubleshoot the issue based on the error output r10r shared here.

[silvanocerza]

I don't think we should go out of our way to provide support for non-compliant projects, but that we should also accept their occurrence as an inevitability. In this particular case, UIPEthernet is a very popular library and the version issue was even fixed in a branch 7 years ago, but the library author never made that branch default and seems to have abandoned the library.

In Arduino Lint, we now have a dedicated tool for encouraging specification compliance, so it's reasonable to make Arduino CLI resilient to a hostile environment when doing so doesn't have a significantly harmful effect on the maintainability or functionality.

Yeah, I think the same.