Add c-lightning module for differential fuzzing by erickcestari · Pull Request #149 · bitcoinfuzz/bitcoinfuzz

erickcestari · 2025-04-10T19:08:32Z

This commit introduces a c-lightning module to the bitcoinfuzz project, enabling differential fuzzing of lightning invoices.

Key changes:

Module Integration:

Add modules/clightning directory with module implementation and wrapper
Compile c-lightning as a library and link it to the clightning module.cpp file

Build System:

Update GitHub workflow to include c-lightning module in fuzzing
Modify Makefile to handle c-lightning dependencies

Differential Fuzzing:

Enhance driver to compare invoice deserialization across implementations
Enable detection of discrepancies between c-lightning and other modules

Dependencies:

Add c-lightning as a submodule in external/lightning
Configure necessary build dependencies

Closes #96

morehouse

Approach ACK. I'll take a closer look at the details next week.

morehouse · 2025-04-11T20:59:56Z

modules/clightning/Makefile-clightning

@@ -0,0 +1,27 @@
+LIB_NAME = libcln.a
+
+$(LIB_NAME): libccan.a $(BITCOIN_OBJS) $(COMMON_OBJS) $(WIRE_OBJS) $(EXTERNAL_LIBS)


Nice! I think this will be less likely to break than if we maintained a full copy of CLN's Makefile.

Still a little brittle though, since CLN can always change their Makefile in a way that would break us. Ideally this libcln.a target would become part of CLN's Makefile upstream, but I'm not sure they'll want to maintain something they don't use themselves.

Regardless, I think it's fine to append here for now.

morehouse

Main feedback is that we should use libFuzzer instrumentation for libcln.a, which may help to find more bugs.

Most other comments are nits.

Makefile

modules/clightning/Makefile

modules/clightning/Makefile-clightning

modules/clightning/clightning_lib.cpp

modules/clightning/module.cpp

modules/clightning/Makefile-clightning

modules/clightning/module.cpp

morehouse

Code LGTM. Haven't tested.

brunoerg · 2025-04-28T12:47:26Z

I'm getting the following error when building c-lightning module (I'm following exactly the steps from the documentation):

➜  clightning git:(149) ✗ make
Appending Makefile-clightning to /Users/brunogarcia/projects/bitcoinfuzz/external/lightning/Makefile if not already present...
Marker not found, appending...
Building lightning in /Users/brunogarcia/projects/bitcoinfuzz/external/lightning...
cd /Users/brunogarcia/projects/bitcoinfuzz/external/lightning && make libcln.a
Makefile:403: warning: overriding commands for target `&'
Makefile:307: warning: ignoring old commands for target `&'
Makefile:1065: *** multiple target patterns.  Stop.
make: *** [libcln.a] Error 2

erickcestari · 2025-04-28T13:45:02Z

I'm getting the following error when building c-lightning module (I'm following exactly the steps from the documentation):

➜  clightning git:(149) ✗ make
Appending Makefile-clightning to /Users/brunogarcia/projects/bitcoinfuzz/external/lightning/Makefile if not already present...
Marker not found, appending...
Building lightning in /Users/brunogarcia/projects/bitcoinfuzz/external/lightning...
cd /Users/brunogarcia/projects/bitcoinfuzz/external/lightning && make libcln.a
Makefile:403: warning: overriding commands for target `&'
Makefile:307: warning: ignoring old commands for target `&'
Makefile:1065: *** multiple target patterns.  Stop.
make: *** [libcln.a] Error 2

It's probably because macOS handles the cat command a little differently from Linux. This should be fixed now.

brunoerg · 2025-04-28T17:45:53Z

It's probably because macOS handles the cat command a little differently from Linux. This should be fixed now.

Thanks, now i'm getting other errors:

Appending Makefile-clightning to /Users/brunogarcia/projects/bitcoinfuzz/external/lightning/Makefile if not already present...
Makefile already includes clightning makefile content.
Building lightning in /Users/brunogarcia/projects/bitcoinfuzz/external/lightning...
cd /Users/brunogarcia/projects/bitcoinfuzz/external/lightning && make libcln.a
Makefile:403: warning: overriding commands for target `&'
Makefile:307: warning: ignoring old commands for target `&'
cc ccan/ccan/closefrom/closefrom.c
ccan/ccan/closefrom/closefrom.c:83:2: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
   83 |         sprintf(dnam, "/proc/%ld/fd", (long) getpid());
      |         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk/usr/include/stdio.h:180:1: note: 'sprintf' has been explicitly marked deprecated here
  180 | __deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
      | ^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk/usr/include/sys/cdefs.h:215:48: note: expanded from macro '__deprecated_msg'
  215 |         #define __deprecated_msg(_msg) __attribute__((__deprecated__(_msg)))
      |                                                       ^
ccan/ccan/closefrom/closefrom.c:162:2: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
  162 |         sprintf(dnam, "/proc/%ld/fd", (long) getpid());
      |         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk/usr/include/stdio.h:180:1: note: 'sprintf' has been explicitly marked deprecated here
  180 | __deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
      | ^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk/usr/include/sys/cdefs.h:215:48: note: expanded from macro '__deprecated_msg'
  215 |         #define __deprecated_msg(_msg) __attribute__((__deprecated__(_msg)))
      |                                                       ^
2 errors generated.
make[1]: *** [ccan-closefrom.o] Error 1
make: *** [libcln.a] Error 2

It's probably an upstream issue related to my configs, will take a better look at c-lightning documentation. By the way, I thought that CFLAGS="-Wno-deprecated-declarations" could fix it but I get another error:

Appending Makefile-clightning to /Users/brunogarcia/projects/bitcoinfuzz/external/lightning/Makefile if not already present...
Makefile already includes clightning makefile content.
Building lightning in /Users/brunogarcia/projects/bitcoinfuzz/external/lightning...
cd /Users/brunogarcia/projects/bitcoinfuzz/external/lightning && make libcln.a CFLAGS="-Wno-deprecated-declarations"
Makefile:403: warning: overriding commands for target `&'
Makefile:307: warning: ignoring old commands for target `&'
cc ccan/ccan/closefrom/closefrom.c
ccan/ccan/closefrom/closefrom.c:2:10: fatal error: 'ccan/closefrom/closefrom.h' file not found
    2 | #include <ccan/closefrom/closefrom.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[1]: *** [ccan-closefrom.o] Error 1
make: *** [libcln.a] Error 2

brunoerg · 2025-04-28T20:40:29Z

modules/lnd/lnd_wrapper/wrapper.go

 	if invoice.MilliSat != nil {
 		sb.WriteString(fmt.Sprintf("%d", *invoice.MilliSat))
+	} else {
+		sb.WriteString("0")


Commit adab97d says "Add c-lightning module for differential fuzzing" but it's touching the lnd module as well. In any similar case, it would be great to split the commits and explain it but no need to do this here now, just a tip for future work.

brunoerg

ACK adab97d; but needs rebase

brunoerg · 2025-05-02T18:00:03Z

From CI:

Building lightning in /Users/runner/work/bitcoinfuzz/bitcoinfuzz/external/lightning...
cd /Users/runner/work/bitcoinfuzz/bitcoinfuzz/external/lightning && make libcln.a
Makefile:387: warning: overriding commands for target `&'
Makefile:307: warning: ignoring old commands for target `&'
Makefile:403: warning: overriding commands for target `&'
Makefile:387: warning: ignoring old commands for target `&'
cc ccan/ccan/asort/asort.c
cc ccan/ccan/base64/base64.c
cc ccan/ccan/bitmap/bitmap.c
cc ccan/ccan/bitops/bitops.c
cc ccan/ccan/breakpoint/breakpoint.c
cc ccan/ccan/closefrom/closefrom.c
ccan/ccan/closefrom/closefrom.c:83:2: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
   83 |         sprintf(dnam, "/proc/%ld/fd", (long) getpid());
      |         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/usr/include/stdio.h:188:1: note: 'sprintf' has been explicitly marked deprecated here
  188 | __deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
      | ^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/usr/include/sys/cdefs.h:215:48: note: expanded from macro '__deprecated_msg'
  215 |         #define __deprecated_msg(_msg) __attribute__((__deprecated__(_msg)))
      |                                                       ^
ccan/ccan/closefrom/closefrom.c:162:2: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
  [162](https://github.com/bitcoinfuzz/bitcoinfuzz/actions/runs/14800305053/job/41557259861?pr=149#step:23:163) |         sprintf(dnam, "/proc/%ld/fd", (long) getpid());
      |         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/usr/include/stdio.h:188:1: note: 'sprintf' has been explicitly marked deprecated here
  188 | __deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
      | ^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/usr/include/sys/cdefs.h:215:48: note: expanded from macro '__deprecated_msg'
  215 |         #define __deprecated_msg(_msg) __attribute__((__deprecated__(_msg)))
      |                                                       ^
2 errors generated.
make[1]: *** [ccan-closefrom.o] Error 1
make: *** [libcln.a] Error 2

Basically same errors that I'm getting on my macOS machine.

erickcestari · 2025-05-02T18:47:59Z

From CI:

Building lightning in /Users/runner/work/bitcoinfuzz/bitcoinfuzz/external/lightning...
cd /Users/runner/work/bitcoinfuzz/bitcoinfuzz/external/lightning && make libcln.a
Makefile:387: warning: overriding commands for target `&'
Makefile:307: warning: ignoring old commands for target `&'
Makefile:403: warning: overriding commands for target `&'
Makefile:387: warning: ignoring old commands for target `&'
cc ccan/ccan/asort/asort.c
cc ccan/ccan/base64/base64.c
cc ccan/ccan/bitmap/bitmap.c
cc ccan/ccan/bitops/bitops.c
cc ccan/ccan/breakpoint/breakpoint.c
cc ccan/ccan/closefrom/closefrom.c
ccan/ccan/closefrom/closefrom.c:83:2: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
   83 |         sprintf(dnam, "/proc/%ld/fd", (long) getpid());
      |         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/usr/include/stdio.h:188:1: note: 'sprintf' has been explicitly marked deprecated here
  188 | __deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
      | ^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/usr/include/sys/cdefs.h:215:48: note: expanded from macro '__deprecated_msg'
  215 |         #define __deprecated_msg(_msg) __attribute__((__deprecated__(_msg)))
      |                                                       ^
ccan/ccan/closefrom/closefrom.c:162:2: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
  [162](https://github.com/bitcoinfuzz/bitcoinfuzz/actions/runs/14800305053/job/41557259861?pr=149#step:23:163) |         sprintf(dnam, "/proc/%ld/fd", (long) getpid());
      |         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/usr/include/stdio.h:188:1: note: 'sprintf' has been explicitly marked deprecated here
  188 | __deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
      | ^
/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk/usr/include/sys/cdefs.h:215:48: note: expanded from macro '__deprecated_msg'
  215 |         #define __deprecated_msg(_msg) __attribute__((__deprecated__(_msg)))
      |                                                       ^
2 errors generated.
make[1]: *** [ccan-closefrom.o] Error 1
make: *** [libcln.a] Error 2

Basically same errors that I'm getting on my macOS machine.

It's probably missing some dependencies. The CLN CI has a separate setup just for macOS. I'll check it out and fix it.

https://github.com/ElementsProject/lightning/blob/master/.github/workflows/macos.yaml

brunoerg

About https://github.com/bitcoinfuzz/bitcoinfuzz/actions/runs/14861350413/job/41726772609?pr=149. Bitcoin Core conflicts with c-lightning due to libsec but they will not be used together, even that I believe we should support building this way and I'm going to fix it. However, to not block this PR I suggest to simply clean Bitcoin Core before running any Lightning Network fuzz target.

erickcestari · 2025-05-06T18:48:40Z

About https://github.com/bitcoinfuzz/bitcoinfuzz/actions/runs/14861350413/job/41726772609?pr=149. Bitcoin Core conflicts with c-lightning due to libsec but they will not be used together, even that I believe we should support building this way and I'm going to fix it. However, to not block this PR I suggest to simply clean Bitcoin Core before running any Lightning Network fuzz target.

Shouldn't the same error occur on Ubuntu as well? I've made a slight change in how we're compiling for macOS by using different configure parameters. When I tried to compile with clang on macOS, I encountered other errors during the compilation stage. Perhaps for a follow-up PR, we should consider using clang for macOS builds to properly instrument the code like we do on Ubuntu.

$(LIGHTNING_DIR)/config.vars:
	@echo "Running ./configure in $(LIGHTNING_DIR)..."
	@if [ "$(shell uname)" = "Darwin" ]; then \
		cd $(LIGHTNING_DIR) && ./configure --disable-compat --disable-valgrind CC=cc; \
	else \
		cd $(LIGHTNING_DIR) && ./configure --enable-address-sanitizer --disable-compat --enable-fuzzing --disable-valgrind CC=clang; \
	fi

This commit introduces a c-lightning module to the bitcoinfuzz project, enabling differential fuzzing of lightning invoices. Key changes: Module Integration: - Add `modules/clightning` directory with module implementation and wrapper - Compile c-lightning as a library and link it to the clightning module.cpp file Build System: - Update GitHub workflow to include c-lightning module in fuzzing - Modify Makefile to handle c-lightning dependencies Differential Fuzzing: - Enhance driver to compare invoice deserialization across implementations - Enable detection of discrepancies between c-lightning and other modules Dependencies: - Add c-lightning as a submodule in `external/lightning` - Configure necessary build dependencies

brunoerg

ACK 0c3b6f2

erickcestari mentioned this pull request Apr 10, 2025

Add c-lightning module for differential fuzzing - DRAFT #132

Closed

brunoerg requested a review from morehouse April 10, 2025 21:42

morehouse reviewed Apr 11, 2025

View reviewed changes

morehouse suggested changes Apr 15, 2025

View reviewed changes

erickcestari force-pushed the clightning-static branch 4 times, most recently from f575968 to d32fb8d Compare April 16, 2025 11:23

erickcestari requested a review from morehouse April 16, 2025 11:37

erickcestari force-pushed the clightning-static branch 2 times, most recently from e4387f0 to 7720bf7 Compare April 16, 2025 12:08

erickcestari mentioned this pull request Apr 16, 2025

Support building Core Lightning as a static library for C/C++ integration ElementsProject/lightning#8240

Open

morehouse reviewed Apr 22, 2025

View reviewed changes

modules/clightning/Makefile-clightning Outdated Show resolved Hide resolved

erickcestari force-pushed the clightning-static branch from 7720bf7 to 3b9a13c Compare April 22, 2025 18:27

morehouse approved these changes Apr 22, 2025

View reviewed changes

modules/clightning/module.cpp Outdated Show resolved Hide resolved

erickcestari force-pushed the clightning-static branch from 3b9a13c to 3e8d0c5 Compare April 23, 2025 11:28

morehouse approved these changes Apr 23, 2025

View reviewed changes

erickcestari force-pushed the clightning-static branch from 3e8d0c5 to 8b6ab8e Compare April 25, 2025 14:54

erickcestari force-pushed the clightning-static branch from 8b6ab8e to adab97d Compare April 28, 2025 13:42

brunoerg reviewed Apr 28, 2025

View reviewed changes

brunoerg approved these changes May 2, 2025

View reviewed changes

erickcestari force-pushed the clightning-static branch from adab97d to b05f6ba Compare May 2, 2025 17:37

erickcestari force-pushed the clightning-static branch from b05f6ba to 1b7c8b1 Compare May 6, 2025 13:47

brunoerg reviewed May 6, 2025

View reviewed changes

erickcestari added 2 commits May 6, 2025 15:52

ci: clean bitcoin core module to avoid linkage error

0c3b6f2

erickcestari force-pushed the clightning-static branch from 178a6f8 to 0c3b6f2 Compare May 6, 2025 18:53

brunoerg approved these changes May 6, 2025

View reviewed changes

brunoerg merged commit 09f8d8d into bitcoinfuzz:v2 May 6, 2025
2 checks passed

		@@ -0,0 +1,27 @@
		LIB_NAME = libcln.a

		$(LIB_NAME): libccan.a $(BITCOIN_OBJS) $(COMMON_OBJS) $(WIRE_OBJS) $(EXTERNAL_LIBS)

Conversation

erickcestari commented Apr 10, 2025

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

morehouse Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

brunoerg commented Apr 28, 2025

Uh oh!

erickcestari commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brunoerg commented Apr 28, 2025

Uh oh!

brunoerg Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

brunoerg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brunoerg commented May 2, 2025

Uh oh!

erickcestari commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brunoerg left a comment

Choose a reason for hiding this comment

Uh oh!

erickcestari commented May 6, 2025

Uh oh!

brunoerg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

erickcestari commented Apr 28, 2025 •

edited

Loading

brunoerg left a comment •

edited

Loading

erickcestari commented May 2, 2025 •

edited

Loading