Skip to content

Commit b995a23

Browse files
fix(api): Disallow creating courses with non alphanumeric (+-_) and long slug
This fixes a security issue, where a malicious lecturer or admin, or attacker who gained their access can create a course with a slug that is injected into commands, a worker runs. Executing just `ffmpeg` in the worker has been considered, but the worker currently relies on the shells capability to redirect its stdout to a file. Thanks to @oiidmnk and @lcarilla for reporting the issue!
1 parent 35dbd1d commit b995a23

File tree

4 files changed

+44
-1
lines changed

4 files changed

+44
-1
lines changed

model/course.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package model
22

33
import (
44
"encoding/json"
5+
"errors"
56
"fmt"
67
"log"
8+
"regexp"
79
"time"
810

911
"gorm.io/gorm"
@@ -331,3 +333,13 @@ func (c Course) IsLoggedIn() bool {
331333
func (c Course) IsEnrolled() bool {
332334
return c.Visibility == "enrolled"
333335
}
336+
337+
var courseSlugRegex = regexp.MustCompile(`^[a-zA-Z0-9\-_]{1,150}$`)
338+
339+
// BeforeSave returns an error if the course to be inserted is invalid
340+
func (c *Course) BeforeSave(tx *gorm.DB) (err error) {
341+
if !courseSlugRegex.MatchString(c.Slug) {
342+
return errors.New("invalid course slug")
343+
}
344+
return nil
345+
}

model/course_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package model
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
func TestBeforeSave(t *testing.T) {
9+
course := &Course{}
10+
cases := []struct {
11+
slug string
12+
wantErr bool
13+
}{
14+
{"", true},
15+
{strings.Repeat("a", 300), true},
16+
{"test123", false},
17+
{"test_123", false},
18+
{"!test", true},
19+
{"\" && rm -rf /", true},
20+
}
21+
for _, tc := range cases {
22+
course.Slug = tc.slug
23+
err := course.BeforeSave(nil)
24+
if err == nil && tc.wantErr {
25+
t.Errorf("BeforeCreate(%q) = nil, want error", tc.slug)
26+
} else if err != nil && !tc.wantErr {
27+
t.Errorf("BeforeCreate(%q) = %v, want nil", tc.slug, err)
28+
}
29+
}
30+
}

model/lecture_hall_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package model

web/template/admin/admin_tabs/create-course.gohtml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
required
5050
placeholder="eidi"
5151
:class="slug === '' ? 'border-red-500 focus:border-red-500' : ''"
52-
@input="slug = slug.replace(/[^A-Za-z0-9\-_.+()~]/g, '')"/>
52+
@input="slug = slug.replace(/[^A-Za-z0-9\-_]/g, '')"/>
5353
</div>
5454
{{template "semester-selection"}}
5555
</form>

0 commit comments

Comments
 (0)