Skip to content

Commit

Permalink
sanity check emoji inputs (keybase#23453)
Browse files Browse the repository at this point in the history
* sanity check emoji inputs

* min length check

* tweaks

* fix tests

* review feedback

* check err
  • Loading branch information
joshblum authored Apr 4, 2020
1 parent e3e832a commit 5f6228b
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 27 deletions.
99 changes: 85 additions & 14 deletions go/chat/emojisource.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,34 @@ import (
"context"
"errors"
"fmt"
"image/gif"
"io/ioutil"
"os"
"strings"
"sync"
"time"

"camlistore.org/pkg/images"
"github.com/keybase/client/go/chat/attachments"
"github.com/keybase/client/go/chat/globals"
"github.com/keybase/client/go/chat/storage"
"github.com/keybase/client/go/chat/types"
"github.com/keybase/client/go/chat/utils"
"github.com/keybase/client/go/libkb"
"github.com/keybase/client/go/protocol/chat1"
"github.com/keybase/client/go/protocol/gregor1"
"github.com/keybase/client/go/protocol/keybase1"
"github.com/kyokomi/emoji"
)

const (
minShortNameLength = 2
maxShortNameLength = 48
minEmojiSize = 512 // min size for reading mime type
maxEmojiSize = 256 * 1000 // 256kb
minEmojiWidth = 16
minEmojiHeight = 16
maxEmojiWidth = 128
maxEmojiHeight = 128
)

type DevConvEmojiSource struct {
Expand Down Expand Up @@ -91,28 +104,86 @@ func (s *DevConvEmojiSource) addAdvanced(ctx context.Context, uid gregor1.UID,
}

func (s *DevConvEmojiSource) isStockEmoji(alias string) bool {
_, ok := emoji.CodeMap()[":"+alias+":"]
if !ok {
_, ok = emoji.CodeMap()[":"+strings.ReplaceAll(alias, "-", "_")+":"]
alias = fmt.Sprintf(":%s:", alias)
alias2 := strings.ReplaceAll(alias, "-", "_")
return storage.EmojiExists(alias) || storage.EmojiExists(alias2)
}

func (s *DevConvEmojiSource) validateShortName(shortName string) (string, error) {
shortName = strings.ReplaceAll(shortName, ":", "") // drop any colons from alias
if s.isStockEmoji(shortName) {
return "", errors.New("cannot use existing stock emoji short name")
}
if len(shortName) > maxShortNameLength || len(shortName) < minShortNameLength {
return "", fmt.Errorf("short name %q (length %d) not within bounds %d,%d",
shortName, len(shortName), minShortNameLength, maxShortNameLength)
}
return ok
if strings.Contains(shortName, "#") {
return "", errors.New("invalid character in emoji alias")
}
return shortName, nil
}

func (s *DevConvEmojiSource) validateAlias(alias string) (string, error) {
alias = strings.ReplaceAll(alias, ":", "") // drop any colons from alias
if strings.Contains(alias, "#") {
return alias, errors.New("invalid character in emoji alias")
func (s *DevConvEmojiSource) validateCustomEmoji(ctx context.Context, shortName, filename string) (string, error) {
shortName, err := s.validateShortName(shortName)
if err != nil {
return "", err
}
if s.isStockEmoji(alias) {
return alias, errors.New("cannot use existing stock emoji alias")

err = s.validateFile(ctx, filename)
if err != nil {
return "", err
}
return alias, nil
return shortName, nil
}

// validateFile validates the following:
// file size
// dimensions
// format
func (s *DevConvEmojiSource) validateFile(ctx context.Context, filename string) error {
finfo, err := attachments.StatOSOrKbfsFile(ctx, s.G().GlobalContext, filename)
if err != nil {
return err
}
if finfo.IsDir() {
return errors.New("invalid file type for emoji")
} else if finfo.Size() > maxEmojiSize || finfo.Size() < minEmojiSize {
return fmt.Errorf("emoji size %d not within bounds %d,%d", finfo.Size(), minEmojiSize, maxEmojiSize)
}

src, err := attachments.NewReadCloseResetter(ctx, s.G().GlobalContext, filename)
if err != nil {
return err
}
defer func() { src.Close() }()
img, _, err := images.Decode(src, nil)
if err != nil {
if err := src.Reset(); err != nil {
return err
}
g, err := gif.DecodeAll(src)
if err != nil {
return err
}
if len(g.Image) == 0 {
return errors.New("no image frames in GIF")
}
img = g.Image[0]
}
bounds := img.Bounds()
if bounds.Dx() > maxEmojiWidth || bounds.Dx() < minEmojiWidth ||
bounds.Dy() > maxEmojiHeight || bounds.Dy() < minEmojiHeight {
return fmt.Errorf("invalid dimensions %dx%d not within %dx%d, %dx%d",
bounds.Dx(), bounds.Dy(), maxEmojiWidth, maxEmojiHeight, minEmojiWidth, minEmojiHeight)
}
return nil
}

func (s *DevConvEmojiSource) Add(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID,
alias, filename string) (res chat1.EmojiRemoteSource, err error) {
defer s.Trace(ctx, func() error { return err }, "Add")()
if alias, err = s.validateAlias(alias); err != nil {
if alias, err = s.validateCustomEmoji(ctx, alias, filename); err != nil {
return res, err
}
storage := s.makeStorage(chat1.TopicType_EMOJI)
Expand All @@ -122,7 +193,7 @@ func (s *DevConvEmojiSource) Add(ctx context.Context, uid gregor1.UID, convID ch
func (s *DevConvEmojiSource) AddAlias(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID,
newAlias, existingAlias string) (res chat1.EmojiRemoteSource, err error) {
defer s.Trace(ctx, func() error { return err }, "AddAlias")()
if newAlias, err = s.validateAlias(newAlias); err != nil {
if newAlias, err = s.validateShortName(newAlias); err != nil {
return res, err
}
var stored chat1.EmojiStorage
Expand Down
4 changes: 2 additions & 2 deletions go/chat/emojisource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestEmojiSourceBasic(t *testing.T) {
uploader := attachments.NewUploader(tc.Context(), store, mockSigningRemote{},
func() chat1.RemoteInterface { return ri }, 1)
tc.ChatG.AttachmentUploader = uploader
filename := "./testdata/ship.jpg"
filename := "./testdata/party_parrot.gif"

conv := mustCreateConversationForTest(t, ctc, users[0], chat1.TopicType_CHAT,
chat1.ConversationMembersType_IMPTEAMNATIVE)
Expand Down Expand Up @@ -262,7 +262,7 @@ func TestEmojiSourceCrossTeam(t *testing.T) {
uploader := attachments.NewUploader(tc.Context(), store, mockSigningRemote{},
func() chat1.RemoteInterface { return ri }, 1)
tc.ChatG.AttachmentUploader = uploader
filename := "./testdata/ship.jpg"
filename := "./testdata/party_parrot.gif"
t.Logf("uid1: %s", uid1)

aloneConv := mustCreateConversationForTest(t, ctc, users[0], chat1.TopicType_CHAT,
Expand Down
22 changes: 11 additions & 11 deletions go/chat/storage/reacjis.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,24 @@ const (
// If the user has less than 5 favorite reacjis we stuff these defaults in.
var DefaultTopReacjis = []string{":+1:", ":-1:", ":joy:", ":sunglasses:", ":tada:"}

// RevCodeMap gets the underlying map of emoji.
func RevCodeMap() map[string][]string {
return emojiRevCodeMap
}

func AliasList(shortCode string) []string {
func EmojiAliasList(shortCode string) []string {
return emojiRevCodeMap[emojiCodeMap[shortCode]]
}

// HasAlias flags if the given `shortCode` has multiple aliases with other
// EmojiHasAlias flags if the given `shortCode` has multiple aliases with other
// codes.
func HasAlias(shortCode string) bool {
return len(AliasList(shortCode)) > 1
func EmojiHasAlias(shortCode string) bool {
return len(EmojiAliasList(shortCode)) > 1
}

// EmojiExists flags if the given `shortCode` is a valid emoji
func EmojiExists(shortCode string) bool {
return len(EmojiAliasList(shortCode)) > 0
}

// NormalizeShortCode normalizes a given `shortCode` to a deterministic alias.
func NormalizeShortCode(shortCode string) string {
shortLists := AliasList(shortCode)
shortLists := EmojiAliasList(shortCode)
if len(shortLists) == 0 {
return shortCode
}
Expand Down Expand Up @@ -236,7 +236,7 @@ func (s *ReacjiStore) populateCacheLocked(ctx context.Context, uid gregor1.UID)
func (s *ReacjiStore) PutReacji(ctx context.Context, uid gregor1.UID, shortCode string) error {
s.Lock()
defer s.Unlock()
if !(HasAlias(shortCode) || globals.EmojiPattern.MatchString(shortCode)) {
if !(EmojiHasAlias(shortCode) || globals.EmojiPattern.MatchString(shortCode)) {
return nil
}
cache := s.populateCacheLocked(ctx, uid)
Expand Down
Binary file added go/chat/testdata/party_parrot.gif
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 5f6228b

Please sign in to comment.