From 66a9d6d2bbc2bc642624236b2f38d1bf2a552899 Mon Sep 17 00:00:00 2001
From: MatteoPologruto <109663225+MatteoPologruto@users.noreply.github.com>
Date: Tue, 6 Feb 2024 09:33:48 +0100
Subject: [PATCH] [skip-changelog] Improve the definition of FQBN and
 explicitly state which characters are allowed (#2509)

* Improve the definition of FQBN and explicitly state which characters are allowed

* Enforce fqbn characters validation

* Test that an error is returned when the FQBN contains an invalid character
---
 docs/FAQ.md                         |  6 +++++-
 internal/arduino/cores/fqbn.go      | 16 ++++++++++++++++
 internal/arduino/cores/fqbn_test.go | 15 +++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/docs/FAQ.md b/docs/FAQ.md
index a58c40098d5..84599a2bbbf 100644
--- a/docs/FAQ.md
+++ b/docs/FAQ.md
@@ -9,7 +9,11 @@ When you run [`arduino-cli board list`][arduino cli board list], your board does
 
 ## What's the FQBN string?
 
-For a deeper understanding of how FQBN works, you should understand the [Arduino platform specification][0].
+FQBN stands for Fully Qualified Board Name. It has the following format:
+`VENDOR:ARCHITECTURE:BOARD_ID[:MENU_ID=OPTION_ID[,MENU2_ID=OPTION_ID ...]]`, with each `MENU_ID=OPTION_ID` being an
+optional key-value pair configuration. Each field accepts letters (`A-Z` or `a-z`), numbers (`0-9`), underscores (`_`),
+dashes(`-`) and dots(`.`). The special character `=` is accepted in the configuration value. For a deeper understanding
+of how FQBN works, you should understand the [Arduino platform specification][0].
 
 ## How to set multiple board options?
 
diff --git a/internal/arduino/cores/fqbn.go b/internal/arduino/cores/fqbn.go
index c5b4384ae5d..8c6ef74ea39 100644
--- a/internal/arduino/cores/fqbn.go
+++ b/internal/arduino/cores/fqbn.go
@@ -17,6 +17,7 @@ package cores
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 
 	properties "github.com/arduino/go-properties-orderedmap"
@@ -57,6 +58,13 @@ func ParseFQBN(fqbnIn string) (*FQBN, error) {
 	if fqbn.BoardID == "" {
 		return nil, fmt.Errorf(tr("empty board identifier"))
 	}
+	// Check if the fqbn contains invalid characters
+	fqbnValidationRegex := regexp.MustCompile(`^[a-zA-Z0-9_.-]*$`)
+	for i := 0; i < 3; i++ {
+		if !fqbnValidationRegex.MatchString(fqbnParts[i]) {
+			return nil, fmt.Errorf(tr("fqbn's field %s contains an invalid character"), fqbnParts[i])
+		}
+	}
 	if len(fqbnParts) > 3 {
 		for _, pair := range strings.Split(fqbnParts[3], ",") {
 			parts := strings.SplitN(pair, "=", 2)
@@ -68,6 +76,14 @@ func ParseFQBN(fqbnIn string) (*FQBN, error) {
 			if k == "" {
 				return nil, fmt.Errorf(tr("invalid config option: %s"), pair)
 			}
+			if !fqbnValidationRegex.MatchString(k) {
+				return nil, fmt.Errorf(tr("config key %s contains an invalid character"), k)
+			}
+			// The config value can also contain the = symbol
+			valueValidationRegex := regexp.MustCompile(`^[a-zA-Z0-9=_.-]*$`)
+			if !valueValidationRegex.MatchString(v) {
+				return nil, fmt.Errorf(tr("config value %s contains an invalid character"), v)
+			}
 			fqbn.Configs.Set(k, v)
 		}
 	}
diff --git a/internal/arduino/cores/fqbn_test.go b/internal/arduino/cores/fqbn_test.go
index 5c4cd9836f8..53200cabaca 100644
--- a/internal/arduino/cores/fqbn_test.go
+++ b/internal/arduino/cores/fqbn_test.go
@@ -155,3 +155,18 @@ func TestMatch(t *testing.T) {
 		require.False(t, b.Match(a))
 	}
 }
+
+func TestValidCharacters(t *testing.T) {
+	// These FQBNs contain valid characters
+	validFqbns := []string{"ardui_no:av_r:un_o", "arduin.o:av.r:un.o", "arduin-o:av-r:un-o", "arduin-o:av-r:un-o:a=b=c=d"}
+	for _, fqbn := range validFqbns {
+		_, err := ParseFQBN(fqbn)
+		require.NoError(t, err)
+	}
+	// These FQBNs contain invalid characters
+	invalidFqbns := []string{"arduin-o:av-r:un=o", "arduin?o:av-r:uno", "arduino:av*r:uno"}
+	for _, fqbn := range invalidFqbns {
+		_, err := ParseFQBN(fqbn)
+		require.Error(t, err)
+	}
+}