fix: add proper HTML escaping in renderFormField

Enhances template safety by escaping user-provided values before inserting
them into HTML output. Improves the function by consistently using escaped
variables throughout the implementation.

- Adds template.HTMLEscapeString for all dynamic values
- Updates variable naming for consistency (escapedName, idAttr, etc.)
- Adds tests to verify proper character escaping works as expected

Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
This commit is contained in:
Ville Vesilehto 2025-03-21 17:51:54 +02:00 committed by Ming Deng
parent 1f40a88b0c
commit 939bb18c66
2 changed files with 71 additions and 10 deletions

View File

@ -314,35 +314,53 @@ func RenderForm(obj interface{}) template.HTML {
// renderFormField returns a string containing HTML of a single form field. In case of select fType, it will retrun // renderFormField returns a string containing HTML of a single form field. In case of select fType, it will retrun
// select tag with options. Value for select fType must be comma separated string which are use are // select tag with options. Value for select fType must be comma separated string which are use are
func renderFormField(label, name, fType string, value interface{}, id string, class string, required bool) string { func renderFormField(label, name, fType string, value interface{}, id string, class string, required bool) string {
// Format attributes with spaces first
idAttr := ""
if id != "" { if id != "" {
id = " id=\"" + id + "\"" idAttr = " id=\"" + template.HTMLEscapeString(id) + "\""
} }
classAttr := ""
if class != "" { if class != "" {
class = " class=\"" + class + "\"" classAttr = " class=\"" + template.HTMLEscapeString(class) + "\""
} }
requiredString := "" requiredAttr := ""
if required { if required {
requiredString = " required" requiredAttr = " required"
}
// Escape all string values
escapedName := template.HTMLEscapeString(name)
escapedLabel := template.HTMLEscapeString(label)
escapedType := template.HTMLEscapeString(fType)
// Handle value specially as it's an interface{}
escapedValue := ""
if value != nil {
escapedValue = template.HTMLEscapeString(fmt.Sprintf("%v", value))
} }
if isValidForInput(fType) { if isValidForInput(fType) {
return fmt.Sprintf(`%v<input%v%v name="%v" type="%v" value="%v"%v>`, label, id, class, name, fType, value, requiredString) return fmt.Sprintf(`%v<input%v%v name="%v" type="%v" value="%v"%v>`,
escapedLabel, idAttr, classAttr, escapedName, escapedType, escapedValue, requiredAttr)
} }
if fType == "select" { if fType == "select" {
valueStr, ok := value.(string) rawValueStr, ok := value.(string)
if !ok { if !ok {
logs.Error("for select value must comma separated string that are the options for select") logs.Error("for select value must comma separated string that are the options for select")
return "" return ""
} }
var selectBuilder strings.Builder var selectBuilder strings.Builder
selectBuilder.WriteString(fmt.Sprintf(`%v<select%v%v name="%v"></br>`, label, id, class, name)) selectBuilder.WriteString(fmt.Sprintf(`%v<select%v%v name="%v"></br>`,
escapedLabel, idAttr, classAttr, escapedName))
for _, option := range strings.Split(valueStr, ",") { for _, option := range strings.Split(rawValueStr, ",") {
selectBuilder.WriteString(fmt.Sprintf(` <option value="%v"> %v </option></br>`, option, option)) escapedOption := template.HTMLEscapeString(option)
selectBuilder.WriteString(fmt.Sprintf(` <option value="%v"> %v </option></br>`,
escapedOption, escapedOption))
} }
selectBuilder.WriteString(`</select>`) selectBuilder.WriteString(`</select>`)
@ -350,7 +368,8 @@ func renderFormField(label, name, fType string, value interface{}, id string, cl
return selectBuilder.String() return selectBuilder.String()
} }
return fmt.Sprintf(`%v<%v%v%v name="%v"%v>%v</%v>`, label, fType, id, class, name, requiredString, value, fType) return fmt.Sprintf(`%v<%v%v%v name="%v"%v>%v</%v>`,
escapedLabel, escapedType, idAttr, classAttr, escapedName, requiredAttr, escapedValue, escapedType)
} }
// isValidForInput checks if fType is a valid value for the `type` property of an HTML input element. // isValidForInput checks if fType is a valid value for the `type` property of an HTML input element.

View File

@ -18,6 +18,7 @@ import (
"html/template" "html/template"
"net/url" "net/url"
"reflect" "reflect"
"strings"
"testing" "testing"
"time" "time"
) )
@ -583,3 +584,44 @@ func Test_lt(t *testing.T) {
} }
} }
} }
func TestRenderFormSecurity(t *testing.T) {
type UserProfile struct {
DisplayName string `form:"displayName,text,Name:"`
Bio string `form:",textarea"`
}
// Test case 1: Test proper escaping of special characters in attributes
specialCharsProfile := UserProfile{
DisplayName: `Special " ' < > & Characters`,
Bio: "Normal text content",
}
output := string(RenderForm(&specialCharsProfile))
// Verify the output has all special characters properly escaped
if strings.Contains(output, `"Special "`) {
t.Errorf("Quotation mark not properly escaped in attribute")
}
if !strings.Contains(output, `value="Special`) {
t.Errorf("Expected escaped attribute value not found")
}
// Test case 2: Test proper escaping of HTML-like content
htmlContentProfile := UserProfile{
DisplayName: "Normal Name",
Bio: `<div>Sample HTML content</div>`,
}
output = string(RenderForm(&htmlContentProfile))
// Verify the output has HTML content properly escaped
if strings.Contains(output, `<div>`) {
t.Errorf("HTML tags not properly escaped in content")
}
if !strings.Contains(output, `&lt;div&gt;`) {
t.Errorf("Expected escaped HTML not found")
}
}